completion: misc fixes

[casperdcl]

• update shtab dependency (>=1.0.2)
• hide/skip certain subcommands (e.g. daemon)
  • shtab>=1.0.0 completion: autogenerate #3921 (comment)
• hide/skip certain arguments & options
  • shtab>=1.0.0 <- hide SUPPRESSed options shtab#3
• fix zsh subcommands with hyphens -
  • shtab>=1.0.2 <- zsh: subcommands containing - shtab#4
• re-introduce original metavar names prior to completion: autogenerate #3921
• part of completion: misc updates #4070
• continues completion: autogenerate #3921

[efiop]

So a hack for now 🙁 Is there a possibility to make shtab not affect this in the future?

[casperdcl]

Not a hack? Argparse always adds [] around options and {} around choices, and uses dest as the display name. metavar is the mechanism provided for the purpose of overriding all of this, and was used a lot in the codebase already.

The 29 places where choices were added in #3921 also needed metavar (added here in #4076) to control display.

There are 2 options:

1. make shtab infer completion type (e.g. DVC_FILE) based on dest (e.g. target)

   • decided against this as it forces devs to be consistent with their own naming conventions

2. explicitly mark completion types
   a) add_argument(dest, choices=[shtab.<TYPE>], metavar=dest)

   • current approach
   • dropping metavar adds {} around the display name. The display name itself is easily controllable via shtab.<TYPE>.__str__.
   • shtab.<TYPE> compares True to everything

   b) add_argument(shtab.Complete(dest, "<TYPE>"))

   • shtab.Complete inherits from str

I prefer (1) followed by (2a) and then (2b), but let me know your thoughts

cc @iterative/engineering

[efiop]

@casperdcl I mean that this metavar overwrite is only needed because we use shtab, we didn't use metavars before. Ideally, shtab shouldn't require the users to use such hacks, but at the same time this not that bad, just need to document it. Maybe it could even be used to our advantage if we could make the auto-generated metavar look better, but I'm not even sure it is possible.

[casperdcl]

When you say "auto-generated metavar" what do you mean?

1. what's displayed with add_argument("target", choices=[DVC_FILE])
   • currently {DVCFile}, could be {any-thing|*.dvc|can't avoid {} though}
2. what's displayed with add_argument("target", choices=[DVC_FILE], metavar="target")
   • target
3. what could be displayed if we subclass & override the add_argument method to auto-fill metavar?

[efiop]

@casperdcl I mean add_argument("target"), that would result in target. But when we start using shtab, we get {DVCFile...} or url [{directory?}](which is absolutely confusing), which is a side-effect. Just wondering if we could avoid the side-effect or, possibly, embrace it and try to make it look nice and be helpful. Maybe the latter could be achieved by making shtab choices have their own nice-looking metavars, not sure.

[casperdcl]

I mean add_argument("target"), that would result in target.

yes

But when we start using shtab, we get {DVCFile...}

That never happens (previously or currently).

Instead, with add_argument("target", choices=[DVC_FILE], ...), we used to get {DVCFile}, [{DVCFile}], or [{DVCFile} [{DVCFile} ...]] for required, optional, and optional list, respectively.

Now, with add_argument("target", choices=[DVC_FILE], metavar="DVCFile"), we get DVCFile, [DVCFile], or [DVCFile [DVCFile ...]], respectively, thus restoring the original look (without the {}).

or [{directory?}]

This was the case with add_argument("--optional", choices=[OPTIONAL_DIR]).

Now, with add_argument("--optional", choices=[OPTIONAL_DIR], metavar="directory"), we get [directory], restoring the original look (without {}). It also suppresses shtab's ? feature which arguably could be removed (there's no need for shtab to expressly mark optionals when argparse already does so by adding []).

The options we have are:

1. previous: use choices to specify type, accepting the extra {}
2. current: use choices to specify type and metavar to suppress {}
3. use add_argument(ShtabString("target", type=DVC_FILE))

In all proposed options, we have trivial full control over outputting DVCFile and ? (i.e. it takes one line of code to globally change [{DVCFile?}] to [{DVCFile}] or [{*.dvc|dvc.yml}]).

which is a side-effect.

There are 3 side-effects so I don't know which you refer to:

1. added {}
2. added ?
3. rename target -> DVCFile

Side-effects (2) and (3) are trivially globally configurable by one line of code, side-effect (1) is a built-in argparse feature if using choices.

[Suor]

So the question is - can we make shtab to not use choices?

[casperdcl]

So the question is - can we make shtab to not use choices?

The only way I can see is the third option above with:

add_argument(ShtabString("target", type=DVC_FILE))

where ShtabString is just a str with an extra type parameter

[casperdcl]

For the record: #4148 uses add_argument("target").complete = DVC_FILE