[patch] _hg: completion for 'hg bookmarks'

[patch] _hg: completion for 'hg bookmarks'

[Christoph Mathys]

Hi there

Below is a patch to add completion support for the "hg bookmarks" command.

thg,
Christoph

PS Please CC on reply, I'm not subscribed to the list.

diff --git a/Completion/Unix/Command/_hg b/Completion/Unix/Command/_hg
index e7c21b9..edd7f0d 100644
--- a/Completion/Unix/Command/_hg
+++ b/Completion/Unix/Command/_hg
@@ -962,4 +962,14 @@ _hg_cmd_strip() {
   ':revision:_hg_tags'
 }

+_hg_cmd_bookmarks() {
+  _arguments -s -w : $_hg_global_opts \
+  - '(--force -f)'{-f,--force}'[force]' \
+    '(--rev -r)'{-r+,--rev}'[set bookmark at revision]:revision:_hg_tags' \
+    '(--delete -d)'{-d,--delete}'[delete a given bookmark]' \
+    '(--rename -m)'{-m+,--rename}'[rename given
bookmark]:bookmark:_hg_bookmarks_internal' \
+    ':bookmark:_hg_bookmarks_internal' \
+  - '(--inactive -i)'{-i,--inactive}'[mark a bookmark inactive]'
+}
+
 _hg "$@"

Re: [patch] _hg: completion for 'hg bookmarks'

[Daniel Shahaf]

Christoph Mathys wrote on Thu, Sep 24, 2015 at 16:23:51 +0200:
> Hi there
> 
> Below is a patch to add completion support for the "hg bookmarks" command.
> 

Thanks!  A few comments:

1. The function should be defined further up in the file (_hg_cmd_* are
defined alphabetically).

2. --rev and --message need to become --rev= and --message= to allow
both '--rev ARG' and '--rev=ARG' (which both work).

3. Why do you have a leading '-' on the first and last line?  Is it
intentional?

4. How should positional arguments (i.e., 'hg bookmarks <TAB>') be
completed?  They are currently completed as files; presumably they
should be completed as _hg_bookmarks_internal?

5. Your mailer munged whitespace in the patch.  You should be able to
avoid this by sending the patch as an attachment named *.txt.

Thanks,

Daniel

> thg,
> Christoph
> 
> PS Please CC on reply, I'm not subscribed to the list.
> 
> diff --git a/Completion/Unix/Command/_hg b/Completion/Unix/Command/_hg
> index e7c21b9..edd7f0d 100644
> --- a/Completion/Unix/Command/_hg
> +++ b/Completion/Unix/Command/_hg
> @@ -962,4 +962,14 @@ _hg_cmd_strip() {
>    ':revision:_hg_tags'
>  }
> 
> +_hg_cmd_bookmarks() {
> +  _arguments -s -w : $_hg_global_opts \
> +  - '(--force -f)'{-f,--force}'[force]' \
> +    '(--rev -r)'{-r+,--rev}'[set bookmark at revision]:revision:_hg_tags' \
> +    '(--delete -d)'{-d,--delete}'[delete a given bookmark]' \
> +    '(--rename -m)'{-m+,--rename}'[rename given
> bookmark]:bookmark:_hg_bookmarks_internal' \
> +    ':bookmark:_hg_bookmarks_internal' \
> +  - '(--inactive -i)'{-i,--inactive}'[mark a bookmark inactive]'
> +}
> +
>  _hg "$@"

Re: [patch] _hg: completion for 'hg bookmarks'

[Bart Schaefer]

On Sep 24,  4:05pm, Daniel Shahaf wrote:
}
} 3. Why do you have a leading '-' on the first and last line?  Is it
} intentional?

     It is possible to specify multiple sets of options and arguments
     with the sets separated by single hyphens.  The specifications
     before the first hyphen (if any) are shared by all the remaining
     sets.  The first word in every other set provides a name for the
     set which may appear in exclusion lists in specifications, either
     alone or before one of the possible values described above.  In
     the second case a `-' should appear between this name and the
     remainder.

[...]

     If the name given for one of the mutually exclusive sets is of the
     form `(NAME)' then only one value from each set will ever be
     completed; more formally, all specifications are mutually
     exclusive to all other specifications in the same set.  This is
     useful for defining multiple sets of options which are mutually
     exclusive and in which the options are aliases for each other.

Re: [patch] _hg: completion for 'hg bookmarks'

[Daniel Shahaf]

Bart Schaefer wrote on Thu, Sep 24, 2015 at 09:29:37 -0700:
> On Sep 24,  4:05pm, Daniel Shahaf wrote:
> }
> } 3. Why do you have a leading '-' on the first and last line?  Is it
> } intentional?
> 
>      It is possible to specify multiple sets of options and arguments
>      with the sets separated by single hyphens.  The specifications

I don't see why _hg_cmd_bookmarks() needs to use sets, and in any case,
the documentation you quote suggests set names are mandatory, but Christoph's
patch doesn't name the sets.

Re: [patch] _hg: completion for 'hg bookmarks'

[Christoph Mathys]

[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]

Hi Daniel,

Thank you for the quick response.

On Thu, Sep 24, 2015 at 6:05 PM, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:

> 1. The function should be defined further up in the file (_hg_cmd_* are
> defined alphabetically).

Fixed.

> 2. --rev and --message need to become --rev= and --message= to allow
> both '--rev ARG' and '--rev=ARG' (which both work).

Fixed for --rev.

> 3. Why do you have a leading '-' on the first and last line?  Is it
> intentional?

They were intentional. I wanted to create groups of mutually exclusive
arguments. But on typing the reply I realized that it is very well
possible to call '--inactive <bookmark>', so the patch was wrong. I
have done away with the '-' in this version of the patch and it should
be more straightforward like this.

> 4. How should positional arguments (i.e., 'hg bookmarks <TAB>') be
> completed?  They are currently completed as files; presumably they
> should be completed as _hg_bookmarks_internal?

Should now always be _hg_bookmarks_internal.

> 5. Your mailer munged whitespace in the patch.  You should be able to
> avoid this by sending the patch as an attachment named *.txt.

Done.

thx,
Christophq

[-- Attachment #2: _hg_cmd_bookmarks.diff --]
[-- Type: text/plain, Size: 883 bytes --]

diff --git a/Completion/Unix/Command/_hg b/Completion/Unix/Command/_hg
index 9dd0236..c84faf3 100644
--- a/Completion/Unix/Command/_hg
+++ b/Completion/Unix/Command/_hg
@@ -460,6 +460,16 @@ _hg_cmd_bisect() {
   '(--command -c --noupdate -U)'{-U,--noupdate}'[do not update to target]'
 }
 
+_hg_cmd_bookmarks() {
+  _arguments -s -w : $_hg_global_opts \
+  '(--force -f)'{-f,--force}'[force]' \
+  '(--rev -r)'{-r+,--rev=}'[set bookmark at revision]:revision:_hg_tags' \
+  '(--delete -d)'{-d,--delete}'[delete a given bookmark]' \
+  '(--rename -m)'{-m+,--rename}'[rename given bookmark]:bookmark:_hg_bookmarks_internal' \
+  '(--inactive -i)'{-i,--inactive}'[mark a bookmark inactive]' \
+  ':bookmark:_hg_bookmarks_internal'
+}
+
 _hg_cmd_branch() {
   _arguments -s -w : $_hg_global_opts \
   '(--force -f)'{-f,--force}'[set branch name even if it shadows an existing branch]' \

Re: [patch] _hg: completion for 'hg bookmarks'

[Daniel Shahaf]

Christoph Mathys wrote on Fri, Sep 25, 2015 at 09:14:46 +0200:
> On Thu, Sep 24, 2015 at 6:05 PM, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > 2. --rev and --message need to become --rev= and --message= to allow
> > both '--rev ARG' and '--rev=ARG' (which both work).
> 
> Fixed for --rev.
> 

I meant to say --rename, not --message.  I'll change --rename to
--rename= (they both work).

> thx,
> Christophq

Thanks for fixing all the issues.  I've just noticed another one, which
I missed in my first review: _arguments should be called without the -w
flag.  (I'm quoting the man page from 5.1.1 since it changed recently:)

              -w     In combination with -s, allow option stacking even if one
                     or more of the options take arguments.  For  example,  if
                     -x  takes an argument, with no -s, `-xy' is considered as
                     a single (unhandled) option; with -s, -xy  is  an  option
                     with  the  argument  `y'; with both -s and -w, -xy may be
                     the option -x and the option -y with arguments  still  to
                     come.

So I removed the -w and pushed the patch.  Possible further improvements
are to remove the -w, and change --foo to --foo=, throughout the rest of
_hg.  (You don't have to do this additional patch, but you're welcome to
if you want.)

Thanks for the patches, Christoph!

Daniel

Re: [patch] _hg: completion for 'hg bookmarks'

[Daniel Shahaf]

Daniel Shahaf wrote on Fri, Sep 25, 2015 at 23:18:50 +0000:
> Possible further improvements are to remove the -w, and change --foo
> to --foo=, throughout the rest of _hg.

I had a few minutes so did this in 92584634d3d39e9ca64475ae5af8010e2ccebe24.
(Not posting the diff since it's a mechanical change.)  If I went
overzealous with the global search/replace please let me know.

Cheers,

Daniel