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

CLI help updates #4317

Merged
merged 5 commits into from
Aug 24, 2020
Merged

CLI help updates #4317

merged 5 commits into from
Aug 24, 2020

Conversation

dra27
Copy link
Member

@dra27 dra27 commented Aug 19, 2020

Best checked commit-by-commit:

  • Use Cmdliner.Manpage.s_ constants instead of hard-coded section names (small tidy-up, although possibly one "bug" found)
  • Aliases put the --help and --version options in OPTIONS instead of COMMON OPTIONS
  • opam lock --help was missing all the common information (ENVIRONMENT, BUGS, etc.)

There are two other things to consider/fix:

  • We call the section ENVIRONMENT VARIABLES (OpamArg.help_sections) but the standard name is ENVIRONMENT (Cmdliner.Manpage.s_environment) - any reason not to change this?
  • opam lock has a section What is changed in the lock file? - this should be in capitals and re-worded. Perhaps LOCK PROCEDURE, perhaps?

The --help and --version options are documented in COMMON OPTIONS for
most commands but were documented in OPTIONS for command aliases!
@dra27 dra27 added this to the 2.1.0~beta milestone Aug 19, 2020
@dra27
Copy link
Member Author

dra27 commented Aug 19, 2020

1e5c401 FWIW should go to 2.0 as well.

@rjbou
Copy link
Collaborator

rjbou commented Aug 19, 2020

On the environment variable, no reason to not switch to Cmdliner.Manpage.s_environment.
For opam lock, it is not really the procedure, but what contains the lock file regarding the regular opam file, maybe something like LOCK FILE CONTENT?

master_changes.md Outdated Show resolved Hide resolved
@dra27
Copy link
Member Author

dra27 commented Aug 19, 2020

How about LOCK FILE UPDATES? "CONTENT" to me implied that it completely writes the file, but I (think?!) the point is that it makes changes to it, right?

@rjbou
Copy link
Collaborator

rjbou commented Aug 19, 2020

ok with that, or a more specific LOCK FILE UPDATED FIELDS, to not let think that it is updates on the lock file process, but the fields that are updated from the opam file.

@AltGr
Copy link
Member

AltGr commented Aug 24, 2020

We call the section ENVIRONMENT VARIABLES (OpamArg.help_sections) but the standard name is ENVIRONMENT (Cmdliner.Manpage.s_environment) - any reason not to change this?

Only reason I could see is the completion scripts that parse the manpage output; but they don't use this section so it should be fine.

@AltGr AltGr merged commit 0b72b9f into ocaml:master Aug 24, 2020
@dra27 dra27 deleted the cli-updates branch September 7, 2020 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants