Page MenuHomePhabricator

help: adding a topic on flags
ClosedPublic

Authored by rdamazio on Oct 31 2017, 12:02 AM.

Details

Summary

This is a short topic to explain how command-line flags can be specified.

Some users have been confused by hg offerring different flag syntax than some
other libraries, so it'd be nice to point them to this rather than explaining
it every time.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

rdamazio created this revision.Oct 31 2017, 12:02 AM

We're in code freeze for approximately two more days, so this will have to wait until ~Thursday. Still, thanks for the patch. I'm happy to include something like this. Things that struck me as missing:

  1. Long vs short form (that e.g. "--verbose" and "-v" are equivalent if the refer to the same flag, which they do)
  2. Non-list, non-boolean flags that take arguments, such as "--rev", and that "--rev=tip", "--rev tip", "-r=tip", "-r tip", and "-rtip" are equivalent
  3. Short flags can be combined: "-pvr tip" is valid
  4. Later flags override earlier flags

We don't necessarily have to include all that in the first patch.

dlax added a subscriber: dlax.Oct 31 2017, 4:25 AM
dlax added inline comments.
mercurial/help/flags.txt
31

hg help config says that "defaults" are deprecated and that aliases should be used instead.

av6 added a subscriber: av6.Nov 2 2017, 7:43 AM
av6 added inline comments.
mercurial/help/flags.txt
8

This "lists of strings" caught my attention, but it took me some time to figure out what's the matter. I don't feel strongly, but here are some points:

  1. it's not clear what a "list of strings" is and how to specify it: is it a comma-separated list?
  2. it's not a type that one command-line flag can take (i.e. no --flag=foo,bar or anything), you must use multiple flags on command line to build this list, so it's unlike the 3 previous types
  3. I don't think we have anywhere (in the code) a list of integers or booleans right now, but nothing prevents this in future, or in an extension

I think this data type is a result of looking at the internals that allow some python variables (associated with handling command line flags in the code) to be lists, but because our CLI is not python, this phrase is more distracting than helpful. I think taking the note that's in hg help and explaining it is better: "[+] can be repeated" (essentially merging "​Specifying list flags" and "Overriding flag defaults").

36

This is a weird-looking macro. Does it work when rendered into e.g. HTML (can be seen hgweb)? I've only seen this format:

:hg:`help config`
50

There are also global flags, they are hidden by default too. I guess you can put them into these 3 categories, but I don't think of --quiet or --verbose as advanced (much less experimental or deprecated). Just a nitpick.

rdamazio marked 4 inline comments as done.Nov 8 2017, 10:29 AM
rdamazio updated this revision to Diff 3334.

We're in code freeze for approximately two more days, so this will have to wait until ~Thursday. Still, thanks for the patch. I'm happy to include something like this. Things that struck me as missing:

  1. Long vs short form (that e.g. "--verbose" and "-v" are equivalent if the refer to the same flag, which they do)
  2. Non-list, non-boolean flags that take arguments, such as "--rev", and that "--rev=tip", "--rev tip", "-r=tip", "-r tip", and "-rtip" are equivalent
  3. Short flags can be combined: "-pvr tip" is valid
  4. Later flags override earlier flags

We don't necessarily have to include all that in the first patch.

Tried to address all of these.

mercurial/help/flags.txt
8

That's the whole point of the "Specifying list flags" section below, and in fact what motivated me to send this patch.
I don't think the code supports parsing a list as numbers or booleans, right now.

36

Assume that I don't know what I'm doing, as far as the format of this file is concerned :) I just tried hgweb and fixed a few things.

50

--verbose is used to show those flags in hg help, it's not that that flag itself is advanced.

martinvonz added inline comments.Nov 8 2017, 12:52 PM
mercurial/help/flags.txt
9

The final "and can be specified" sounds a little truncated. Was there supposed to be something after? Or maybe it's no longer needed since you already said "can be used with any command"?

37–40

-ffoo is also valid

68

Do we recommend overriding the command like this or should the left side be called something else (maybe "icommit")? I really don't know what we recommend here, so don't take my question to imply that you shouldn't do it the way you have done it.

87

should be committemp here

95

s/simplify/simply/

rdamazio marked 4 inline comments as done.Nov 9 2017, 6:56 AM
rdamazio updated this revision to Diff 3359.
rdamazio marked 4 inline comments as done.Nov 9 2017, 6:57 AM
rdamazio added inline comments.
mercurial/help/flags.txt
68

That's my interpretation of what "hg help commit" says, but if you want me to change it let me know.

durin42 accepted this revision as: durin42.Nov 10 2017, 5:16 PM
durin42 added a subscriber: durin42.

I'm +1 on this, but won't accept as a reviewer since I've got at least two biases here (it makes permanent my --no- boolean prefix for flags, and it'll solve some support problems at G).

yuja accepted this revision.Nov 13 2017, 8:56 AM
yuja added a subscriber: yuja.

Removed -f=foo and queued, thanks.

it makes permanent my --no- boolean prefix for flags,

Perhaps we can add some note saying --no- is still experimental?

mercurial/help/flags.txt
40

This is wrong. -f=foo is identical to -f =foo.

This revision is now accepted and ready to land.Nov 13 2017, 8:56 AM
This revision was automatically updated to reflect the committed changes.