Skip to content
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

PoC/WIP: Extension to provide an "editable" interface to putup (in opposition to "interactive") #333

Closed
wants to merge 9 commits into from

Conversation

abravalheri
Copy link
Collaborator

In #325 I mentioned that an alternative to "interactive" could be done via a simple text file + an editor:

Imagine that instead of an CLI constantly querying the user, we spawn $EDITOR with an text file with contents similar to putup -h, where users can uncomment flags / review the parameters that they might have already typed directly as arguments. When the file is closed putup runs with it's contents (a kind of 2 passes system, first we preprocess things with cli.parse_args, then we compose the file based on [(a.option_strings, a.help) for a in parser._actions] and leverage argparse's fromfile_prefix_chars and convert_arg_line_to_args to parse the tempfile a second time?).

I wonder how these ergonomics/UX would compare to the interactive mode and how much we can reduce in terms of complexity (e.g. do we even need to overload ArgumentParser this way?)

This PR is a proof of concept in that direction... (more improvement can be done if the extension brings interest)

@FlorianWilhelm
Copy link
Member

Hi @abravalheri, an editable mode is really an out-of-the-box idea. I tried it out and it works good.

What are the pros/cons of an editable mode vs. interactive?

editable

  • easier to implement
  • overview of all flags there are
  • no direct feedback if a flag value was set wrong
  • less familiar to most users

interactive

  • must users are quite familiar with interactive tools
  • direct feedback and reevaluation if typed input is wrong is possible, or ask directly for true/false for instance
  • more convenient to use
  • much harder to implement

I have no strong preference but still feel that for our users an interactive mode would be more suitable. There's got to be a way to implement it :-)

@abravalheri
Copy link
Collaborator Author

Hi @FlorianWilhelm thank you for having a look 😄
I got the inspiration from git and the config files that usually come with examples commented out (like Ngnix, Apache, etc ...).

I agree that the "interactive mode" is something much more users are familiar with, and that using --edit would be something most of the people would see in a CLI tool for the first time (although we can argue that git uses something similar for git rebase -i and for the git commit).

Something that I would add as a con for the "interactive mode" though is that PyScaffold have a lot of options and that asking one at the time might be a problem for impatient people like me 😅 (I counted 20 questions in my dev environment, where I have the external extensions like django, markdown and cookiecutter installed). Maybe if we go for interactive we should curate them?

Another con is that if you by mistake input a wrong option (e.g. when you make a typo and just realise after you press enter, or when another flag helps you to realise the meaning of a previous one), you basically have to do a CTRL+C and start all over (if you are in question number 10 or later, that might be annoying).

A pro for --edit is that currently it pre-processes all the extra options you pass via CLI + things from the config file (global and/or setup.cfg in the case of an update) and automatically changes the generated examples (unless I coded some bugs 😝). So if I type putup --edit --update -f ., the generated file will have --force and --update uncommented + every other configuration from setup.cfg (and $HOME/.config/pyscaffold/default.cfg). Derived options (like name and package) will also have their examples automatically updated (so if I type putup -i myproj, the examples will be --name myproj and --package myproj, instead of --name NAME and --package PACKAGE_NAME). If we go for the "interactive mode", maybe that is something we can copy in the future?

Overall I have the impression the interactive mode is something new comers will instantly feel familiar, but after learning the basics of PyScaffold, they will stop using (20 questions might be a lot). The --edit will be something that will require from new comers an open mind to get used to, but that have some potential to be used even if you are a long term PyScaffold user (specially because it can also be used to update projects, not only to generate new ones, and is handy in those situations, or when you have a bunch of defaults in your personal $HOME/.config/pyscaffold/default.cfg and would like to review them when starting a new project).

@FlorianWilhelm
Copy link
Member

@abravalheri, you do have a point about git interactive and also your points about the config. Maybe we should just try it? So we could have this "editable" feature in version 4.0 if it is so much easier to implement and let's see how users react to it. But I think we should call it -i/--interactive nevertheless as it's done in git.

Still, there would be some points to think about. First wouldn't it be good to have a prompt=False feature nevertheless? Is it really good to show e.g. the --version option in the interface or should this rather have prompt=False? Also when I try the current version in this PR the --config option is activated, i.e. uncommented. But when I run it:

putup: error: argument --config: expected at least one argument

what I find quite counterintuitive. I have to comment something to make it work. Or am I missing something?

… see discussion in pyscaffold#333 for description
@abravalheri
Copy link
Collaborator Author

Hi @FlorianWilhelm thank you very much for the feedback. I fixed the problem with --config, that was definetely a bug 😝.

In the current implementation I am adding some metadata attributes to the extension classes themselves instead of overloading/extending the parser.add_argument method to achieve something similar to prompt=False.

My idea was try to avoid as much as possible inheritance with argparse, because it is hard. So extension writers can declare an on_edit = {"ignore": [...]} or on_edit = {"comment": [..]} attribute to achieve similar results... Not sure if it is an ideal implementation, but I tried to be pragmatic regarding dealing with argparse. (By the way, this is a technique that we could try to copy to the interactive mode if we think it helps to reduce complexity).

I am fine with any naming to be sincere (--edit or --interactive). I guess if it works for git, it should be fine for us. I also accept any suggestions for the short help text 😝

My final remark is that I am currently using 2 protected methods from argparse, similarly to the "interactive mode" (I suppose), parser._actions and parser._get_formatter. We could use inheritance to avoid "breaking the implicit rule" of not using _methods, but I don't see much real benefit on doing that inside a class method or a function in an isolated file. The edit.py file is extremely coupled with the argparse implementation details, and changes in the Python upstream code imply in reviewing the entire file anyway...

@FlorianWilhelm
Copy link
Member

Hi @abravalheri, thanks, I think it's good to keep the coupling with argparse to a mininum. Also, accessing protected members is in general not good, but in this case, I would argue that it is okay due to the circumstances that argparse provides no better interface and is stable even with its inner workings. If they ever decide to redesign argparse they will give at a new name and do it completely over again for backward compatibility (like done with optparse). Nevertheless, one should mention this coupling maybe in the detailed description of the extensions/edit.py module's docstring and make sure that accessing these variables is only done in a minimum of places by having e.g. a function get_parser_actions that just returns parser._actions.

Regarding the short text, would do you think about: interactively choose and configure PyScaffold's parameters?

@abravalheri abravalheri deleted the branch pyscaffold:v4.0.x December 29, 2020 20:46
@abravalheri
Copy link
Collaborator Author

The PR got automatically closed when I merged the v4 branch on master and deleted the branch, but it is still valid 😅

I will open a new one

abravalheri added a commit to abravalheri/pyscaffold that referenced this pull request Dec 29, 2020
… see discussion in pyscaffold#333 for description
abravalheri added a commit to abravalheri/pyscaffold that referenced this pull request Jan 1, 2021
… see discussion in pyscaffold#333 for description
abravalheri added a commit to abravalheri/pyscaffold that referenced this pull request Jan 2, 2021
… see discussion in pyscaffold#333 for description
abravalheri added a commit to abravalheri/pyscaffold that referenced this pull request Jan 3, 2021
… see discussion in pyscaffold#333 for description
abravalheri added a commit that referenced this pull request Jan 3, 2021
… see discussion in #333 for description
CarliJoy pushed a commit to CarliJoy/pyscaffold that referenced this pull request Jan 5, 2021
… see discussion in pyscaffold#333 for description
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants