Improve cli and help by Metaxal · Pull Request #147 · jackfirth/resyntax

Metaxal · 2021-07-31T15:30:00Z

See first commit message below.

previously: $ racket -l- resyntax does nothing, leaving the user unsure what to do. After some code/doc search, the user may try $ racket -l- resyntax/cli but this again does nothing $racket -l- resyntax/cli --help say either "analyze" or "fix" should be used $racket -l- resyntax/cli analyze does again nothing $racket -l- resyntax/cli analyze --help finally provides some useful guidance. new behaviour: $ racket -l- resyntax displays a message to try the following: $ racket -l- resyntax/cli --help Furthermore, all of $ racket -l- resyntax/cli $ racket -l- resyntax/cli --analyze $ racket -l- resyntax/cli --fix default to adding the "--help" flag so as to provide meaningful help. Command line parsing has been changed to look more like hierarchical argument parsing. A `parameterize-help-if-empty-ccla` form makes the command line arguments default to #("--help") if they are empty. The "analyze" and "fix" commands are now parsed properly by `command-line`, which means they must be written "--analyze" and "--fix".

Metaxal · 2021-07-31T15:53:41Z

cli.rkt

-    (define parsed-modpath (read (open-input-string modpath)))
-    (define parsed-suite-name (read (open-input-string suite-name)))
-    (set-box! suite (dynamic-require parsed-modpath parsed-suite-name))))
+  (parameterize-help-if-empty-ccla


Only this line has changed n this function. The rest is indentation.

Metaxal · 2021-07-31T15:54:07Z

cli.rkt

-    (define parsed-modpath (read (open-input-string modpath)))
-    (define parsed-suite-name (read (open-input-string suite-name)))
-    (set-box! suite (dynamic-require parsed-modpath parsed-suite-name))))
+  (parameterize-help-if-empty-ccla


Only this line has changed in this function. The rest is indentation.

jackfirth · 2021-08-06T02:26:58Z

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.

jackfirth · 2021-08-06T02:42:35Z

cli.rkt

+
+;; If the command line arguments are empty, re-parameterize it to
+;; default to #("--help")
+(define-syntax-parse-rule (parameterize-help-if-empty-ccla body ...)


I think we ought to try to add this to command-line as an opt-in feature. So that we could write something like:

(command-line #:program-name "foo" #:print-help-by-default flags ...)

That would certainly be useful.

Metaxal · 2021-08-06T13:17:29Z

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 · 2021-08-07T00:20:44Z

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 · 2021-08-07T11:47:24Z

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 · 2021-08-11T04:52:33Z

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

Metaxal · 2021-08-11T10:04:26Z

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 · 2021-08-12T02:11:51Z

Yes, that would work.

Metaxal · 2021-08-13T12:05:43Z

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 · 2024-09-01T08:41:51Z

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.

Metaxal added 4 commits July 31, 2021 16:17

update cli docs

f011ac0

Use for simpler hierarchical parsing

05002bf

remove dead code, reorganize

83f520c

Metaxal commented Jul 31, 2021

View reviewed changes

jackfirth reviewed Aug 6, 2021

View reviewed changes

jackfirth closed this Sep 1, 2024

Conversation

Metaxal commented Jul 31, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Metaxal Jul 31, 2021

Choose a reason for hiding this comment

Uh oh!

Metaxal Jul 31, 2021

Choose a reason for hiding this comment

Uh oh!

jackfirth commented Aug 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jackfirth Aug 6, 2021

Choose a reason for hiding this comment

Uh oh!

Metaxal Aug 6, 2021

Choose a reason for hiding this comment

Uh oh!

Metaxal commented Aug 6, 2021

Uh oh!

jackfirth commented Aug 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Metaxal commented Aug 7, 2021

Uh oh!

jackfirth commented Aug 11, 2021

Uh oh!

Metaxal commented Aug 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jackfirth commented Aug 12, 2021

Uh oh!

Metaxal commented Aug 13, 2021

Uh oh!

jackfirth commented Sep 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Metaxal commented Jul 31, 2021 •

edited

Loading

jackfirth commented Aug 6, 2021 •

edited

Loading

jackfirth commented Aug 7, 2021 •

edited

Loading

Metaxal commented Aug 11, 2021 •

edited

Loading