feat: implement dict questions#2231
Conversation
|
@sisp any chance for reviewing this? I know it is a quite big implementation, but it will bring copier to the next level in my opinion. Thanks for your awesome work |
|
Thanks for the reminder, @dellekappa! 👍 And sorry for the extreme delay, it did slip my mind! 🫣 I'll try to find some time for a review in the coming days. 🤞 |
sisp
left a comment
There was a problem hiding this comment.
Sorry again for the huge delay! 🫣
Phew, this is a tough one, mainly because Copier's current architecture isn't design for nested questions (which you've addressed) and because I have a different approach for orchestrating the questionnaire flow in mind which likely requires several prior refactoring steps that I'm (slowly 🙁) trying to work towards. In short:
-
We should refactor the single
Questionclass into a hierarchy of semantically distinct question classes:base question ├── text question ├── choice question │ ├── single-choice question │ └── multi-choice question └── confirmation questionAll but the confirmation question support multiple answer value types (e.g.,
str,int,float, etc. – exceptboolwhich is reserved for the confirmation question). This nested/dict question is a "question container" in my mind. -
We should delegate question-asking (interactive prompting, default/programmatic/previous answer use) to the question classes including question containers like nested/dict "questions". For this, all question-like classes must implement a protocol that includes a method like
ask(...)(signature to be decided yet). A "real" question would manage actual answer retrieval while a question container would iterate over its question-like children and call theirask(...)methods. This way, answer retrieval logic is nicely encapsulated. There wouldn't be the explicit concept of inner nodes and leaf nodes, just protocol-implementing questions-like types.
I agree with the dot-path syntax for answers passed via CLI flag, but I think properly nested data for the Python API and answers file would be preferable. dpath seems unmaintained, so I'm not sure I'd be comfortable using it. Regarding the type for this question container, I think we deliberately didn't go with dict to avoid resemblance with Python, as Copier is a (mostly) language-agnostic tool despite being written in Python.
I absolutely acknowledge the work you've put into this PR 🙇, but I'm not sure what kind of actionable feedback I should give TBH. 😕
|
Thanks to take some time taking a look at it and don't worry about your decisions, I know this PR is hard to review and to accept because this implementation, as you correctly noticed, requires an important refactor.
So, essentially a questionary is composed by a list of About Even if this PR will never be merged I hope can be a source of inspiration :) Thanks |
Probably a better implementation for issue #751 compared to PR #2223
This PR relates to issue #751
Some reasoning follows:
implemented the question type dict that will be rendered, as expected, as a dict in jinja context. This is why I used dict instead of nested
on the answers file side, the answers are still a flat list that includes only the leafs of the question tree, but, to be able to traverse the question tree hierarchy, a classical property.property synthax has been used. Under the hood this is handled by dpath. This permits the user to easily specify an answer value from the command line at any hierarchy level.
the deepness in prompt have been rendered just with indentation. Even if what have been proposed in #751 is much more appealing, is also much harder to implement because requires to know in advance the questions that will be rendered