-
-
Notifications
You must be signed in to change notification settings - Fork 186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add the ability to customise how questions are asked #674
Comments
Yes, this seems reasonable. However I'd like to know beforehand what's your suggestion for a less distracting proposal. Maybe we can just hardcode something simpler, as most people won't complain about simplicity. |
My main concerns are:
I think I'd prefer something like: project_name:
type: str
help: Enter your project name
repeat:
type: int giving
So basically, "{emoji} {help_or_var_name}: " If you wanted to include the type, maybe add " [{type}]" just before the colon, but only when it's not "str", so you would have:
The emoji is OK, but it's a bit of a personal preference. If copier had a "user settings" file, I'd suggest letting the user alter the emoji in that, but I don't think it's something you'd want on a command line. So maybe don't worry about it for now. |
Seems good to me. Would you want to volunteer that PR? |
Absolutely, I'd love to 🙂 |
I've reviewed the copier templates under the copier-template topic. There seem to be three common styles of help:
In order to avoid breaking people's existing templates, I propose that for case (1) we use the help with a ": " suffix, in case (2) we use the help unchanged. I have yet to see other forms such as "A description with punctuation!" and I'm not entirely sure what to do with them. For now, I'll defer the problem and just treat them like case (1). For case (3), I think that applying basically the same rule (remove any trailing newlines, then either add a space or ": " depending on where there's a question mark) gives something fairly OK. But I'm a little hesitant here, as the template developers have obviously put some effort into writing useful help, and I don't want to mess that up. What do you think? Also, I have a problem in that a bunch of the tests fail on Windows, and pre-commit complains (the prettier and mixed line ending checks fail, but the diff shows nothing but "LF will be replaced by CRLF" and a couple of "No newline at end of file" changes). I'm not at all sure I know what to do to fix this, and in any case I don't want to mix up this change with a bunch of "try to make things work on Windows" changes. I can switch off the pre-commit hook in order to actually commit my code, but the tests are more of an issue. There seem to be a lot of tests that depend on the question format, and they all use pexpect, which doesn't work on Windows so those tests are set to xfail. I'll see if I can get a Linux development environment set up using WSL, but otherwise I may be able to do the code change, but not fix the existing tests that rely on the existing prompt layout. |
Actually, a much simpler approach here might be to just add a new attribute for a question, "prompt". If the prompt is set, it is used in place of the string f"{self.var_name}? Format: {self.get_type_name()}". This means that the default behaviour remains the same, but template authors can use a different prompt if they want. Would that be acceptable? I've just updated a draft PR (#680) which adds this functionality. No docs or tests yet, I mainly just want to see if my change passes the existing test suite at this point. |
I don't think that adding Regarding all those use cases, the most complex ones are multiline string inputs, which in turn can be formatted and validated as json or yaml inputs. One option I think would be quite good would be to always put a newline before the answer. So, questions template would be something like
If the template author wants to finish with colon, dot, question mark... it's up to them. Also we have selection and boolean questions, and possibly some other... Regarding tests, setting up a WSL dev env should be pretty straightforward. Sadly, Windows is a 2nd class citizen here (I just rely on CI for it). Taking care the pexpect tests would be quite important for this feature to land, I'm afraid. Those took a lot of work to do! |
Oh BTW I just remembered we support Gitpod. Check https://copier.readthedocs.io/en/stable/contributing/#dev-environment-setup |
OK, I've got WSL set up. What I'm trying to do at the moment is refactor the tests so that rather than every test depending on the exact form of the prompt, there's a single helper that "expects" a prompt. That will make changing the prompt much less messy. But I'm hitting an issue with q = ["Wanna give me any more info?", "anything_else", "Format: yaml"]
tui.expect_exact(q)
tui.sendline("- Want some grog?")
tui.expect_exact(invalid + q)
tui.sendline("- I'd love it")
tui.send(Keyboard.Esc + Keyboard.Enter) If I'm reading the documentation of expect_exact correctly, passing a list checks if any one of them matches ("If you pass a list of patterns and more than one matches, the first match in the stream is chosen"). But the code above looks like it's intending to test that the whole list is displayed. I'm certainly not sure enough of how pexpect works to be completely clear on what's happening, but if I'm reading things correctly, the existing code does the following:
So the test passes, but it never actually ensures the "Invalid input" message is displayed. And of course, it shouldn't (because that message isn't displayed) but as written the code seems like it intends to do that check. And the previous checks for "Invalid input" work roughly the same, but in those cases it should be checking that "Invalid input" was displayed, but it isn't actually doing that check... I've fixed the "anything_else" test as part of my refactoring so that it no longer tries to check for "Invalid input", and my helper which checks for the invalid input message does actually make that check (I know precisely because it failed on "anything_else" 🙂). So hopefully the refactoring adds some value beyond just making it easier to modify the prompt. By the way, while looking at this, it occurred to me that most of these tests using pexpect, are in practice just testing that the questionary library does what it's documented as doing. While it's certainly reasonable to want to ensure that the UI looks the way you intend it to, I wonder whether it would be possible to add some tests (which would be supported on Windows, unlike the pexpect tests) that worked by mocking questionary and testing it was called with the right inputs? That would give better test coverage on Windows, while still having the full check that the UI looks as expected via the Unix CI. That's probably a lot of work, and I'm not sure I could commit to it right now, but would it be of interest if I did find the time at some point? |
I've submitted #682 for the refactoring of the tests. If that is acceptable, I can then start on changing the format, and we can see what works best (on reflection, |
Ah! 🤦🏼♂️ I simply misunderstood what
Indeed! It should give some fallback for those cases. |
Regarding this point, I'm not sure that would improve the situation here. The main reason why I decided the current approach is that I wanted to be able to merge blindly dependabot updates, in the case it's updating the TUI-related tests. Also it's easier when doing TDD for the TUI to expect how it should look like and, then, just do the code. About windows coverage, there's https://github.com/raczben/wexpect which should be able to let us abstract the OS and run the "same" TUI tests. And in any case, our coverage report aggregates coverage results from all systems. So yes, windows situation is not the best, but as you said this change would be a ton of work and I'm not sure it'd be worth it. I think that adding support for wexpect would be much more simple and worth it. |
Fantastic. I'd love to help improve Windows support, but I'm cautious of over-committing myself at this point. If there's anything where you feel I could help, though, feel free to ping me 🙂 |
For now the best would be to just try to conditionally use pexpext or wexpect on the TUI tests depending on the OS, and remove the windows xfails. wexpect was recently refactored. Some time ago I tried that and it didn't work, but it might work today. Being more used to windows 🪟 than me, I guess you'll be able to do that more easily. However this is a bit away from this issue's subject so I suggest to keep this thread focused on the prompt stuff and maybe we can open another for the windows coverage problem. |
Getting back on topic, the key point for me with the prompt is that I find the "Format: str" hint extremely distracting. There are a couple of reasons for this:
Apart from that, my main other preference is that I'd like to be able to construct a template so that each question only takes up a single line, so the prompting is compact. But given that I've seen help texts in the form "What is your name?" and "Enter your name", it's hard to come up with a way that produces a single line prompt that looks good in all cases (given that "what looks good" is extremely subjective here...) So my suggestion would be:
This seems reasonable to me, with the only odd case being that if people use a 1-line help, they won't get ">" unless they include it in the help. The alternative would be
This style doesn't give you any way to get a single-line question. Having played with both of these, I find that I like the first one far better. The single line question style just looks much more readable to me. There's still some style choices I'm not sure about (> versus some other character, plus the question of whether to try to intelligently adapt if the help text ends with punctuation) but we can work out those details. What's your view on the basic question of whether it's worth the more complex rule of the first approach in order to allow the single-line style? (By the way, I should say that I've found a gross hack to let a Jinja extension monkeypatch the |
I prefer to add the Try with a questionary like this: # copier.yaml
your_choice:
type: str
multiline: true
help: Choose something
choices:
- a
- b
name:
type: str
help: your name, please
some_list:
type: yaml
multiline: true
help: |
I need you to pass me a list.
Remember: must be a good list
yn:
type: bool
help: want to drink something? This is how it looks right now, FTR: Kooha-05-28-2022-12-56-10.mp4IMHO it would look quite nice if all questions looked pretty much the same, no matter the type of question or the length or lines of text involved in the help attribute. Maybe you can do a quick mockup and send a video like this to see how this questionary looks with your suggestion? |
Here's my first option (single line when possible): Option.1.mp4Here's my second option (always \n>): Option.2.mp4 |
In my personal taste, I like more the 2nd option. Although after seeing it in action, I think I'd just replace that @pawamoy what do you thinhk? |
I like compact stuff. When all questions are single-line, I prefer the compact version. But since questions (and answers!) can be multiline, I think the second version would be better to handle all cases. It also has a clear benefit: the input field has more width. Not sure it's still accurate (maybe questionary handles it fine), but in previous versions, and just like in most shells, if you go past the right threshold, and therefore on a new line, you can't go back to edit (left/up). On mobile right now, will try to expand later. |
I agree there's no ideal answer that makes every possible question look good. That's why I'd originally thought of letting the config file provide a prompt format that works well with the help texts the config uses. But as we don't want to do that, I can see the argument for having something that's consistent, even if it's not particularly compact. I'm happy to go with whatever you prefer at this point. For my personal templates, if I want something compact, I can always use the hack I mentioned above... |
OK then let's advance with the double line proposal.
FWIW in case we wanted to have this configurable, I think it should be configurable per user, not per template. Because it depends more on personal taste, not something the template dev should care about IMHO. The same might happen with colors, fonts... Also some other experience I had in mind was a fullscreen TUI with backwards and forward buttons to let you edit previous answers. But that's probably something I'll never have time to do 😆. |
Possibly. But as we've seen, any given format typically works best with a particular style of help, so there's an argument that it should be tied to the template as well. But as you say, that's something to worry about another day 🙂
Maybe expose an API that can be used by 3rd parties, and let someone else do that for you? LOL, so many ideas, so little time. I'll do a PR in the next couple of days for the prompt change we've agreed. I'll replace the > with 2 spaces as per your earlier message. |
Discussed in #673
Originally posted by pfmoore May 23, 2022
The prompt currently appears to be a 🎤emoji, followed by the help (if specified), then the item name and a format prompt. I'd like to be able to change the prompt as I find it difficult to read (in particular, the format prompt is very distracting, and I'd like to omit it). Is that possible?
Having checked the source code, it seems that this isn't possible. But the
questionary
library looks very flexible, so it would be possible to do this if there was a way to describe the format in the template configuration. Would such a change be welcome? I'd be more than happy to work on a PR for this, but I'm very new to the project (I'm currently trying to migrate my existing cookiecutter templates over to copier) so I don't know if that's something that would be considered a reasonable feature for the project.I'd imagine doing this via settings in the
copier.yml
file, so the format would be decided by the template, rather than by the user. Would that make sense?The text was updated successfully, but these errors were encountered: