jackfirth / resyntax

A Racket refactoring engine
Apache License 2.0
56 stars 10 forks source link

Improve cli and help #147

Closed Metaxal closed 1 week ago

Metaxal commented 3 years ago

See first commit message below.

jackfirth commented 3 years ago

The analyze-mode v.s. fix-mode distinction being a flag is definitely something I want to avoid, as it's very unlikely they'll remain as similar as they are now over time and I really want them to look and feel like distinct operations to users. What do you think about making separate resyntax/cli/analyze and resyntax/cli/fix modules so that the commands are racket -l- resyntax/cli/analyze and racket-l- resyntax/cli/fix? That will at least still avoid the current nasty mess where the command line arguments are parsed twice, with the first time being just to figure out which command to choose.

Metaxal commented 3 years ago

I don't really understand why --fix and --analyze don't work well for the future, and I don't really see the difference with resyntax analyze or resyntax/cli/analyze. Except that at least with the -- way, it's all neatly hierarchically parsed, and you get --help automatically at all levels. Otherwise you'd need to simulate these at the racket -l- resyntax level too, and maybe at sublevels if you decide to have subcommands for --fix and --analyze.

Note that the parsing doesn't do much: it merely looks at the first argument, and the rest is just gobbled up by the commands, so that doesn't seem too heavy to me (it's like first + rest).

But if you really want to split in filse, I would suggest getting rid of /cli altogether so as to write racket -l- resyntax/analyze and racket -l- resyntax/fix directly. And display some help message for racket -l- resyntax to tell what commands to use.

jackfirth commented 3 years ago

The reason I want to avoid --fix and --analyze flags is because I'm certain that eventually we're going to add features to fix-mode that don't make sense for analyze-mode. For example, I'd like to explore adding options to fix-mode to automatically build chains of git commits for fixes. So I'd like to keep them as two separate commands with separate --help text and not have to worry about flags that are only relevant to one of them confusing users reading the help text of the other command.

Re: resyntax/analyze and resyntax/fix: yes I agree, those are much better than resyntax/cli/analyze and resyntax/cli/fix.

Metaxal commented 3 years ago

The reason I want to avoid --fix and --analyze flags is because I'm certain that eventually we're going to add features to fix-mode that don't make sense for analyze-mode. For example, I'd like to explore adding options to fix-mode to automatically build chains of git commits for fixes. So I'd like to keep them as two separate commands with separate --help text and not have to worry about flags that are only relevant to one of them confusing users reading the help text of the other command.

The PR does distinguish subcommand-specific flags:

racket -l- resyntax/cli --help 

says:

usage: resyntax [ <option> ... ]

<option> is one of

/ --analyze
|    Analyze source code without applying changes
| --fix
\    Analyze source code and apply changes
  --help, -h
     Show this help
  --
[...]

and racket -l- resyntax/cli --analyze --help (also racket -l- resyntax/cli --analyze alone) provides help only about the subcommands that are specific to --analyze:

usage: resyntax --analyze [ <option> ... ]

<option> is one of

* --file <filepath>
     A file to analyze.
* --directory <dirpath>
     A directory to anaylze, including subdirectories.
* --package <pkgname>
     An installed package to analyze.
  --refactoring-suite <modpath> <suite-name>
     The refactoring suite to analyze code with.
  --help, -h
     Show this help
  --
[...]

while racket -l- resyntax/cli --fix --help (also racket -l- resyntax/cli --fix) gives:

usage: resyntax --fix [ <option> ... ]

<option> is one of

* --file <filepath>
     A file to fix.
* --directory <dirpath>
     A directory to fix, including subdirectories.
* --package <pkgname>
     An installed package to fix.
  --refactoring-suite <modpath> <suite-name>
     The refactoring suite to analyze code with.
  --help, -h
     Show this help
  --
[...]

Notice that the help texts and usage lines are different, and, even if the flags were different the PR would still produce subcommand-specific help text. It's really hierarchical. (This could actually relatively be easy to turn into an actual hierarchical-cli library.)

[I suppose there's a typo for --fix --refactoring-suite: it should be suite to fix rather than suite to analyze probably?]

jackfirth commented 3 years ago

So resyntax --file foo.rkt --fix wouldn't work? I'd rather not break the intuition that --flag arguments are unordered.

Metaxal commented 3 years ago

Indeed it wouldn't, and the help does say so.

I agree it goes a little against intuition, although there are a number unix command line tools that do break this property eagerly (like zip I just used). I was actually looking at the source code of cmdline, which forces flags to start with - or +. I suppose we could add an option there to allow for plain word tags, in which case you could write racket -l- resyntax analyze --file ... with almost zero change to my code. In principle, would that work for you?

jackfirth commented 3 years ago

Yes, that would work.

Metaxal commented 3 years ago

ok good. Then I'll try to submit a PR to racket/cmdline to allow for non [-+] flags, as well as a PR to racket/cmdline for #:defaults-to-help (in preparation), but I have no idea when I'll have time to devote to this, so don't hold your breath. Then we can think again about merging this one. How does this sound? Let me know if there are other blockers.

jackfirth commented 1 week ago

Closing this since it's a few years old and has conflicts with the current state of things. Let me know if you want to take another stab at this sometime though.