Bugs thrown up by _perforce

Bugs thrown up by _perforce

[Peter Stephenson]

I'm committing another version of _perforce; I don't think there's
enough general interest to warrant posting it, and it's quite a long diff
since I've tried to rationalise names (e.g. changelist/changelists to
changes) as well as add some features (service=p4-<subcommand> works,
dates can be completed) and fixed some bugs (labels and clients after an
@ didn't work).

This has thrown up two quite annoying bugs in the use of `_next_labels'.
The background is that I was trying to use the tag-order style to make
the system first throw up changes (i.e. changeset numbers) after an `@'
(this acts as a modifier to a filename), then the other possibilities.
I ran across two problems.  The second is the real killer --- it makes
_next_tags virtually useless for command arguments.


The first is that _next_tags always accepts a single completion
possibility at the point it's reached (not if it's the initial
possibility).  Here's a suitable set up which will act as the basis for
both bits.

_foos() {
	local arr
	arr=(up)
	_describe -t foos 'type foo' arr
}
_bars() {
	local arr
	arr=(north south east west)
	_describe -t bars 'type bar' arr
}
_foobar() { 
	_alternative foos:foo:_foos bars:bar:_bars
}
compdef _foobar foobar

Completion after `foobar' works fine so far, offering foos and bars
grouped however you ask it to.  Now, let's tell it to offer us foos
before bars:
  zstyle ':completion:*:foobar:*' tag-order foos bars
`foobar ^D' is OK; ^xn for _next_tags works the first time,
switching from foos to bars.  However, when you switch back the `up' is
always completed; you can't switch again.  This is despite the fact that
you are still listing --- the system evidently remembers this, because
with menu completion, nothing is inserted for the bars even though the
list is displayed.

This is a fairly minor annoyance since in most cases like this there
will be more than one completion, and it seems to be possible to delete
back to the place you were in and then call next tags.


The second bug shows up with a similar set up; I'll remove the trigger
for the first bug.  Note the extra level of function calling.

_foos() {
	local arr
	arr=(up down left right)
	_describe -t foos 'type foo' arr
}
_bars() {
	local arr
	arr=(north south east west)
	_describe -t bars 'type bar' arr
}
_foobar_cmd() { 
	_alternative foos:foo:_foos bars:bar:_bars
}
_foobar() {
	_arguments '*::foobar command:_foobar_cmd'
}
compdef _foobar foobar

Again, this works fine with no tag-order defined.  This time, when you
define the tag-order as above, _next_tags fails completely --- there is
no way to move from the foos onto the bars.  Hence my statement above
that this makes _next_tags essentially useless for command arguments.

Altering _foobar to use the state mechanism:

_foobar() {
        local context state line
        typeset -A opt_args
        _arguments '*::foobar command:->foobar_cmd' 

        if [[ $state = foobar_cmd ]]; then
            _foobar_cmd
        fi
}

seems to work around the problem.  It was too much like hard work to get
_perforce to do this this time round.

I won't be looking at the source of these problems since the code for
this sort of thing does my head in.  Unless Oliver or Bart has some
ideas or Sven surfaces we may be a bit stuck.

-- 
Peter Stephenson <pws@pwstephenson.fsnet.co.uk>
Work: pws@csr.com
Web: http://www.pwstephenson.fsnet.co.uk

Re: Bugs thrown up by _perforce

[Oliver Kiddle]

On 23 Feb, Peter wrote:

> This has thrown up two quite annoying bugs in the use of `_next_labels'.

> The first is that _next_tags always accepts a single completion
> possibility at the point it's reached (not if it's the initial
> possibility).  Here's a suitable set up which will act as the basis for
> both bits.
> 
> _foos() {
> 	local arr
> 	arr=(up)
> 	_describe -t foos 'type foo' arr
> }
> _bars() {
> 	local arr
> 	arr=(north south east west)
> 	_describe -t bars 'type bar' arr
> }
> _foobar() { 
> 	_alternative foos:foo:_foos bars:bar:_bars
> }
> compdef _foobar foobar
> 
> Completion after `foobar' works fine so far, offering foos and bars
> grouped however you ask it to.  Now, let's tell it to offer us foos
> before bars:
>   zstyle ':completion:*:foobar:*' tag-order foos bars
> `foobar ^D' is OK; ^xn for _next_tags works the first time,
> switching from foos to bars.  However, when you switch back the `up' is
> always completed; you can't switch again.  This is despite the fact that
> you are still listing --- the system evidently remembers this, because
> with menu completion, nothing is inserted for the bars even though the
> list is displayed.

I don't think it is remembering that it is listing - nothing is
inserted for the bars only because there is no common unambiguous
prefix for them.

You can fix this by changing the ins=unambiguous line to just ins= so
that compstate[insert] remains empty but this changes _next_tags
behaviour in other cases. Possibly for the better: I wonder that the
initial definition of _next_tags shouldn't perhaps be based on
list-choices instead of complete-word (though playing with compstate
mostly overrides that). You might be able to make this configurable with
a style and use things like $LASTWIDGET to see whether we are listing or
completing.

I'm only an occasional _next_tags user so am not sure what the ideal
behaviour would be. Does anyone want _next_tags to actually complete
stuff ever?

> The second bug shows up with a similar set up; I'll remove the trigger
> for the first bug.  Note the extra level of function calling.
> 
> _foos() {
> 	local arr
> 	arr=(up down left right)
> 	_describe -t foos 'type foo' arr
> }
> _bars() {
> 	local arr
> 	arr=(north south east west)
> 	_describe -t bars 'type bar' arr
> }
> _foobar_cmd() { 
> 	_alternative foos:foo:_foos bars:bar:_bars
> }
> _foobar() {
> 	_arguments '*::foobar command:_foobar_cmd'
> }
> compdef _foobar foobar
> 
> Again, this works fine with no tag-order defined.  This time, when you
> define the tag-order as above, _next_tags fails completely --- there is
> no way to move from the foos onto the bars.  Hence my statement above
> that this makes _next_tags essentially useless for command arguments.

I think the problem is that argument-rest is finding its way into
_next_tags_not. You can replace your _foobar() above with just
_foobar() { _wanted foobars expl 'foobar command' _foobar_cmd }
and it still fails the same way so it is nothing to do with _arguments
and command arguments and everything to do with nested tag loops.

Both foos and bars have the argument-rest tag for the first tag loop so
both are excluded by _next_tags.

So what should _next_tags do when we have nested tag loops? Advance inner
before outer? Advance both together? Or something different entirely?

> Altering _foobar to use the state mechanism:

> seems to work around the problem.  It was too much like hard work to get
> _perforce to do this this time round.

That makes sense: _arguments can't do tag loops for states. And, note
that you are ignoring any value set in $context by _arguments here.

> I won't be looking at the source of these problems since the code for
> this sort of thing does my head in.  Unless Oliver or Bart has some
> ideas or Sven surfaces we may be a bit stuck.

Hopefully those ideas at least give us something to work with.

Oliver

This e-mail and any attachment is for authorised use by the intended recipient(s) only.  It may contain proprietary material, confidential information and/or be subject to legal privilege.  It should not be copied, disclosed to, retained or used by, any other party.  If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender.  Thank you.

Re: Bugs thrown up by _perforce

[Peter Stephenson]

Oliver Kiddle wrote:
> [_next_tags always inserts an unambiguous completion]
>
> You can fix this by changing the ins=unambiguous line to just ins= so
> that compstate[insert] remains empty but this changes _next_tags
> behaviour in other cases.

I'll test this out.

> I'm only an occasional _next_tags user so am not sure what the ideal
> behaviour would be. Does anyone want _next_tags to actually complete
> stuff ever?

I can't see why --- certainly not treat it as an unambiguous insertion,
fait accompli.  You can't even see what it's going to complete next.

> [an _arguments --- or any inner tags loop --- screws up _next_tags]
>
> I think the problem is that argument-rest is finding its way into
> _next_tags_not.
> 
> Both foos and bars have the argument-rest tag for the first tag loop so
> both are excluded by _next_tags.
> 
> So what should _next_tags do when we have nested tag loops? Advance inner
> before outer? Advance both together? Or something different entirely?

Um.  I'm still a bit vague as to how all this works.  Can we prevent
this behaviour happening with argument-rest?  After all, it's being
smuggled in behind our back anyway --- it's not specified either in the
styles, nor in the list of tags I'm explicitly generating.

Can we make the system remember we're in a nested loop?  And then inside
it can be more lenient?  Or some combination of the tow?  Or something?

I don't think I'm going to be much help here.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

PATCH: Re: Bugs thrown up by _perforce

[Oliver Kiddle]

Peter wrote:
> Oliver Kiddle wrote:
> > [_next_tags always inserts an unambiguous completion]
> >
> > You can fix this by changing the ins=unambiguous line to just ins= so
> > that compstate[insert] remains empty but this changes _next_tags
> > behaviour in other cases.
> 
> I'll test this out.

That would be useful.

> > behaviour would be. Does anyone want _next_tags to actually complete
> > stuff ever?
> 
> I can't see why --- certainly not treat it as an unambiguous insertion,
> fait accompli.  You can't even see what it's going to complete next.

Okay, I think it can be fixed then.

> > [an _arguments --- or any inner tags loop --- screws up _next_tags]

> Um.  I'm still a bit vague as to how all this works.  Can we prevent
> this behaviour happening with argument-rest?  After all, it's being
> smuggled in behind our back anyway --- it's not specified either in the
> styles, nor in the list of tags I'm explicitly generating.

You could but a more generic solution would be much better. The
argument- tags can be useful.

> Can we make the system remember we're in a nested loop?  And then inside
> it can be more lenient?  Or some combination of the tow?  Or something?

This patch should do the job. It only puts tags from the inner most tag
loops into $_lastcomp[tags] and hence $_next_tags_not. Not blocking an
outer loop tag does not matter because all inner tags will be blocked
for it so the effect is the same.

Does anyone see a problem with the change to what $_lastcomp[tags]
contains? Note that it isn't used by anything other than _next_tags.

Potential problems remain if the same tag names are used across
different branches (the inner loops of separate outer loops). Doing
that is otherwise not very sensible anyway.

Oliver

Index: Completion/Base/Core/_all_labels
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Base/Core/_all_labels,v
retrieving revision 1.2
diff -u -r1.2 _all_labels
--- Completion/Base/Core/_all_labels	12 Feb 2002 13:37:03 -0000	1.2
+++ Completion/Base/Core/_all_labels	26 Feb 2003 08:33:31 -0000
@@ -24,6 +24,8 @@
 fi
 
 while comptags "-A$__prev" "$1" curtag __spec; do
+  (( $#funcstack > _tags_level )) && _comp_tags="${_comp_tags% * }"
+  _tags_level=$#funcstack
   _comp_tags="$_comp_tags $__spec "
   if [[ "$curtag" = *[^\\]:* ]]; then
     zformat -f __descr "${curtag#*:}" "d:$3"
Index: Completion/Base/Core/_main_complete
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Base/Core/_main_complete,v
retrieving revision 1.6
diff -u -r1.6 _main_complete
--- Completion/Base/Core/_main_complete	26 Jun 2002 11:07:46 -0000	1.6
+++ Completion/Base/Core/_main_complete	26 Feb 2003 08:33:31 -0000
@@ -23,6 +23,7 @@
       _matchers _matcher _c_matcher _matcher_num _comp_tags _comp_mesg  \
       mesg str context state line opt_args val_args curcontext="$curcontext" \
       _last_nmatches=-1 _last_menu_style _def_menu_style _menu_style sel \
+      _tags_level=0 \
       _saved_exact="${compstate[exact]}" \
       _saved_lastprompt="${compstate[last_prompt]}" \
       _saved_list="${compstate[list]}" \
Index: Completion/Base/Core/_next_label
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Base/Core/_next_label,v
retrieving revision 1.2
diff -u -r1.2 _next_label
--- Completion/Base/Core/_next_label	12 Feb 2002 13:37:04 -0000	1.2
+++ Completion/Base/Core/_next_label	26 Feb 2003 08:33:31 -0000
@@ -6,6 +6,8 @@
 zparseopts -D -a __gopt 1 2 V J x
 
 if comptags -A "$1" curtag __spec; then
+  (( $#funcstack > _tags_level )) && _comp_tags="${_comp_tags% * }"
+  _tags_level=$#funcstack
   _comp_tags="$_comp_tags $__spec "
   if [[ "$curtag" = *[^\\]:* ]]; then
     zformat -f __descr "${curtag#*:}" "d:$3"
Index: Completion/Base/Widget/_next_tags
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Base/Widget/_next_tags,v
retrieving revision 1.3
diff -u -r1.3 _next_tags
--- Completion/Base/Widget/_next_tags	25 Feb 2003 18:43:51 -0000	1.3
+++ Completion/Base/Widget/_next_tags	26 Feb 2003 08:33:31 -0000
@@ -34,6 +34,8 @@
     fi
 
     while comptags "-A$__prev" "$1" curtag __spec; do
+      (( $#funcstack > _tags_level )) && _comp_tags="${_comp_tags% * }"
+      _tags_level=$#funcstack
       [[ "$_next_tags_not" = *\ ${__spec}\ * ]] && continue
       _comp_tags="$_comp_tags $__spec "
       if [[ "$curtag" = *[^\\]:* ]]; then
@@ -59,6 +61,8 @@
     zparseopts -D -a __gopt 1 2 V J x
 
     if comptags -A "$1" curtag __spec; then
+      (( $#funcstack > _tags_level )) && _comp_tags="${_comp_tags% * }"
+      _tags_level=$#funcstack
       [[ "$_next_tags_not" = *\ ${__spec}\ * ]] && continue
       _comp_tags="$_comp_tags $__spec "
       if [[ "$curtag" = *[^\\]:* ]]; then

This e-mail and any attachment is for authorised use by the intended recipient(s) only.  It may contain proprietary material, confidential information and/or be subject to legal privilege.  It should not be copied, disclosed to, retained or used by, any other party.  If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender.  Thank you.

PATCH: Re: Bugs thrown up by _perforce

[Oliver Kiddle]

On 25 Feb, Peter wrote:
> > [_next_tags always inserts an unambiguous completion]
> >

This patch is my fix for the first of the problems. From my own usage,
I'm happy that this is definitely better. If anyone really does want
_next_tags to insert unambiguous matches, it would be easy to make it
check the insert-unambiguous style.

Oliver

Index: Completion/Base/Widget/_next_tags
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Base/Widget/_next_tags,v
retrieving revision 1.4
diff -u -r1.4 _next_tags
--- Completion/Base/Widget/_next_tags	26 Feb 2003 16:36:07 -0000	1.4
+++ Completion/Base/Widget/_next_tags	11 Mar 2003 12:59:00 -0000
@@ -1,4 +1,4 @@
-#compdef -k complete-word \C-xn
+#compdef -k list-choices \C-xn
 
 # Main widget.
 
@@ -96,11 +96,7 @@
   _next_tags_pfx="$PREFIX"
   _next_tags_sfx="$SUFFIX"
 
-  if [[ -n "$compstate[old_insert]" ]]; then
-    ins=1
-  else
-    ins=unambiguous
-  fi
+  ins="${compstate[old_insert]:+1}"
 
   _main_complete _complete _next_tags_completer
 

Re: PATCH: Re: Bugs thrown up by _perforce

[Peter Stephenson]

Oliver Kiddle wrote:
> On 25 Feb, Peter wrote:
> > > [_next_tags always inserts an unambiguous completion]
> > >
> 
> This patch is my fix for the first of the problems. From my own usage,
> I'm happy that this is definitely better. If anyone really does want
> _next_tags to insert unambiguous matches, it would be easy to make it
> check the insert-unambiguous style.

The current change seems perfectly reasonable.

I finally got around to testing the other fix; it seems to do what I
want now, thanks (at least in _perforce, which is quite a serious test).
This notes the fact in the _perforce comments.

Index: Completion/Unix/Command/_perforce
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Command/_perforce,v
retrieving revision 1.3
diff -u -r1.3 _perforce
--- Completion/Unix/Command/_perforce	23 Feb 2003 16:06:06 -0000	1.3
+++ Completion/Unix/Command/_perforce	12 Mar 2003 11:45:44 -0000
@@ -63,10 +63,13 @@
 # typing @ or # removes the space which was automatically added.
 # The context used has `at-suffix' or `hash-suffix' in the position
 # before the tag to indicate suffix completion (as always, ^Xh will
-# show you all possible contexts).  This should make it possible
+# show you all possible contexts).  This makes it possible
 # to select changes, dates, labels and clients using the tag-order
-# style, but unfortunately there is a bug which stops any tags afer
-# the first group from being displayed.
+# style.  For example,
+#    zstyle ':completion:*:p4-*:at-suffix:*' tag-order changes '*'
+# will force all completion after `@' to show changes first.  Executing
+# _next_tags (usually ^x^n) will cycle between that and the remaining
+# tags (dates, labels, clients).
 #
 # A # is automatically quoted when handled in this way; if the file is
 # typed by hand or the completion didn't finish (e.g. you typed a character

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************