This is an archive of the discontinued Mercurial Phabricator instance.

zsh_completion: update all options
ClosedPublic

Authored by av6 on Aug 11 2018, 12:10 AM.

Details

Summary

It's just too hard to further split this patch. What it does:

  • adds missing flags
  • removes flags that are no longer there
  • updates flag descriptions and argument names
  • adds * where using the same option many times is okay
  • groups with () mutually exclusive flags that can't be used together
  • adds + and = to options that need arguments
  • removes + and = from options that don't take any arguments
  • fixes minor issues and a typo

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

av6 created this revision.Aug 11 2018, 12:10 AM
spectral accepted this revision.Aug 13 2018, 2:58 PM
spectral added a subscriber: spectral.
spectral added inline comments.
contrib/zsh_completion
373

I wonder how hard it would be to add tests to make sure these don't diverge from the actual output in the future? (no action required in this change, just curious if you have any ideas).

At my job our code review system has a "if this is modified, then you probably want to modify this other place" mechanism, I wonder if something like that, generically as part of one of the existing tests (or on its own) would be easier and make more sense than attempting to match the output, just rely on people doing the right thing when the tests prompt them to (with a way of ignoring it, perhaps via something in the commit description). Example:

  1. IfChange @command('^add', walkopts + subrepoopts + dryrunopts, _('[OPTION]... [FILE]...'), inferrrepo=True)
  2. ThenChange(contrib/zsh_completion, contrib/bash_completion) def add(ui, repo, *pats, **opts): ...

Though, since we'd be doing this for every @command, I'm back to thinking it'd probably be better to hard-code it for the completion stuff - something that says "Oh, you've modified the contents of an @command, make sure you update these other locations as well..."

644

I assume the first section here makes continue and abort mutually exclusive? I believe that revision arguments are also forbidden if either --continue or --abort are specified; I don't know if there's a way of representing that.

828

Does the order of the first section matter? Should the three of them be identical?

1198

Does this indicate that hg rebase <hash> -d <dest> should work (without a -r, -s, or -b?) I don't think that's correct..

av6 added inline comments.Aug 14 2018, 12:28 AM
contrib/zsh_completion
644

Just having --rev -r inside the parentheses should be enough, but I'm not sure if implementing every piece of exclusivity logic here makes sense (I'm also concerned about completions diverging from real behavior). But I've found this in zsh's _git:
(- :)--abort[cancel revert or cherry-pick sequence]
This form disables completion of any other arguments, and I think it's future-proof enough to do this for --abort here. I'll do a follow-up to touch just exclusivity when this series lands.

828

I don't think it matters. All options that the group mentions just disappear from completion when -l, -m or -u is present.

1198

You mean giving revisions without -r? No, * means that -r can be specified multiple times (and each would require an argument). To see how it's done for a command that has "optional" -r see _hg_cmd_update or _hg_cmd_strip above ^

spectral added inline comments.Aug 14 2018, 9:34 PM
contrib/zsh_completion
1198

Got it, thanks for explaining :)

This revision was automatically updated to reflect the committed changes.