using the description from _arguments foo:description:->state

using the description from _arguments foo:description:->state

[Mikael Magnusson]

Hi,

When you do something like this (from _zle)

    "($opts)-A[define widget alias]:old widget:->widget :new widget:->widget" \

it ends up not showing those helpful 'old widget' and 'new widget'
because ->widget does this,
_wanted -C "$context[1]" widgets expl widget compadd -k widgets && ret=0

so it just shows "widget" as the description. Is there any way to get
that description from the $state handler and pass it on to compadd?
(I'm a bit afraid to open up _arguments and see if it's passed at
all).

-- 
Mikael Magnusson

Re: using the description from _arguments foo:description:->state

[Bart Schaefer]

On Sep 11,  5:04pm, Mikael Magnusson wrote:
}
}  "($opts)-A[define widget alias]:old widget:->widget :new widget:->widget"
} 
} Is there any way to get that description from the $state handler and
} pass it on to compadd?

It appears the answer to this is categorically "no."

The description is extracted into the $descrs array by the comparguments
builtin called at _arguments line 318.  It's then assigned to $descr at
line 355 and passed through to _description for formatting at line 366.

However, all of the related variables are declared local to _arguments,
so when that eventually returns without ever making a call to compadd,
all that information is discarded.

Given the usual state (ahem) of the _arguments code, it's surprisingly
straightforward to add a parallel array for passing back the description:


Index: Completion/Base/Utility/_arguments
===================================================================
*** Completion/Base/Utility/_arguments	23 Nov 2008 18:26:27 -0000
--- Completion/Base/Utility/_arguments	11 Sep 2011 17:44:45 -0000
***************
*** 344,349 ****
--- 344,350 ----
  
    context=()
    state=()
+   state_descr=()
  
    while true; do
      while _tags; do
***************
*** 376,381 ****
--- 377,383 ----
  	      if (( ! $state[(I)$action] )); then
                  comparguments -W line opt_args
                  state+=( "$action" )
+                 state_descr+=( "$descr" )
  	        if [[ -n "$usecc" ]]; then
  	          curcontext="${oldcontext%:*}:$subc"
  	        else

(Not sure why the use of tabs for indentation is different on adjacent
lines in that section of _arguments.)

However, because callers of _arguments are responsible for declaring
state as a local, there are a HUGE number of files that would have to
be updated to also declare state_descr as a local ... UNLESS we were
willing to declare it local in _main_complete as is done with the
opt_args variable (which is also supposed to be declared local by all
callers of _arguments but conspicuously is not by several completers
including _zle).

Also I'm not really happy with "state_descr" as a name, but I suppose
it'll do.  Anyway then we have:

Index: Completion/Zsh/Command/_zle
===================================================================
--- Completion/Zsh/Command/_zle	21 Dec 2010 16:41:15 -0000
+++ Completion/Zsh/Command/_zle	11 Sep 2011 17:57:16 -0000
@@ -44,7 +44,7 @@
       '(-)*:widget arguments: ' && ret=0
     ;;
   (widget*)
-    _wanted -C "$context[1]" widgets expl widget compadd -k widgets && ret=0
+    _wanted -C "$context[1]" widgets expl "${state_descr[1]:-widget}" compadd -k widgets && ret=0
     ;&
   (function)
     [[ $state[1] != *function ]] ||	# Handle fall-through


Index: Completion/Base/Core/_main_complete
===================================================================
--- Completion/Base/Core/_main_complete	1 Jun 2011 06:39:59 -0000
+++ Completion/Base/Core/_main_complete	11 Sep 2011 17:59:57 -0000
@@ -23,7 +23,8 @@
 local func funcs ret=1 tmp _compskip format nm call match min max i num\
       _completers _completer _completer_num curtag _comp_force_list \
       _matchers _matcher _c_matcher _matcher_num _comp_tags _comp_mesg  \
-      mesg str context state line opt_args val_args curcontext="$curcontext" \
+      mesg str context state state_descr 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]}" \

And for completeness (ahem):

Index: Completion/Base/Utility/_values
===================================================================
RCS file: /extra/cvsroot/zsh/zsh-4.0/Completion/Base/Utility/_values,v
retrieving revision 1.9
diff -c -r1.9 _values
--- Completion/Base/Utility/_values	21 Dec 2010 16:41:15 -0000	1.9
+++ Completion/Base/Utility/_values	11 Sep 2011 18:06:27 -0000
@@ -87,6 +87,7 @@
   if [[ "$action" = -\>* ]]; then
     compvalues -v val_args
     state="${${action[3,-1]##[ 	]#}%%[ 	]#}"
+    state_descr="$descr"
     if [[ -n "$usecc" ]]; then
       curcontext="${oldcontext%:*}:$subc"
     else

Opinions?

-- 
Barton E. Schaefer

Re: using the description from _arguments foo:description:->state

[Mikael Magnusson]

On 11 September 2011 20:10, Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Sep 11,  5:04pm, Mikael Magnusson wrote:
> }
> }  "($opts)-A[define widget alias]:old widget:->widget :new widget:->widget"
> }
> } Is there any way to get that description from the $state handler and
> } pass it on to compadd?

[partial quote]

> Given the usual state (ahem) of the _arguments code, it's surprisingly
> straightforward to add a parallel array for passing back the description:
>
> However, because callers of _arguments are responsible for declaring
> state as a local, there are a HUGE number of files that would have to
> be updated to also declare state_descr as a local ... UNLESS we were
> willing to declare it local in _main_complete as is done with the
> opt_args variable (which is also supposed to be declared local by all
> callers of _arguments but conspicuously is not by several completers
> including _zle).
>
> Opinions?

Can we add a new option to _configure which enables this, so if you
say _arguments -d you also have to local state_descr. There's a couple
of other variables that work like this already, -n for NORMARG, -C for
curcontext.

-- 
Mikael Magnusson

Re: using the description from _arguments foo:description:->state

[Oliver Kiddle]

Mikael Magnusson wrote:
> On 11 September 2011 20:10, Bart Schaefer <schaefer@brasslantern.com>
wrote:
> > }  "($opts)-A[define widget alias]:old widget:->widget :new
widget:->widget"

> Can we add a new option to _configure which enables this, so if you
> say _arguments -d you also have to local state_descr. There's a couple
> of other variables that work like this already, -n for NORMARG, -C for
> curcontext.

While we could, there's always the possibility that some function
somewhere has a -d that's intended to me an argument to be completed.

If you used a function instead of a state, it would be called with the
relevant arguments for the description unless you prefix it with a space
in which case, you'd have to dig around in $expl. Perhaps we could use
some similar syntax to the space prefix instead of an option.

Unless you pass -C to _arguments there is the possibility of more than
one state being possible so any state_descr would need to be an array.
And we can't do arrays of arrays so a state_expl wouldn't be possible
(expl is already an array). That would put me off the whole idea somewhat.

Personally, I'd be inclined to have a separate new-widget state in this
particular function. Alternatively, you could use the value of $context
to get the right description or use a function for completing widgets.

Oliver

Re: using the description from _arguments foo:description:->state

[Bart Schaefer]

On Sep 21,  1:39pm, Oliver Kiddle wrote:
} Subject: Re: using the description from _arguments foo:description:->state
}
} Mikael Magnusson wrote:
} > Can we add a new option to _configure which enables this, so if you
} > say _arguments -d you also have to local state_descr. There's a couple
} > of other variables that work like this already, -n for NORMARG, -C for
} > curcontext.
} 
} While we could, there's always the possibility that some function
} somewhere has a -d that's intended to me an argument to be completed.

That condition is already handled by using e.g. "_arguments -n : -n".

More importantly, I don't think adding -d buys much.  -C changes the
meaning of ->state so that the caller doesn't need to copy from the
context array to curcontext, but there's no equivalent for states or
descriptions (unless we want to overload the name "descr" which is
already used in a bunch of places, but I don't like that idea).

} If you used a function instead of a state, it would be called with the
} relevant arguments for the description unless you prefix it with a space
} in which case, you'd have to dig around in $expl.

Before producing my patch I first thought about digging around in $expl,
but $expl is already marked up for use with _format, so that doesn't
really work.  Also $expl is only passed *down* from _arguments, not back
up to the caller.

} Unless you pass -C to _arguments there is the possibility of more than
} one state being possible so any state_descr would need to be an array.

Yes, that's what my patch does.  $state_descr is an array parallel to
the $state array.  (Except in _values when $state is never an array,
which I find a bit confusing but is probably too late to change.)

} And we can't do arrays of arrays so a state_expl wouldn't be possible
} (expl is already an array). That would put me off the whole idea
} somewhat.

Not an issue, _arguments declares "local expl" so it's never used with
this form of state handling.

} Personally, I'd be inclined to have a separate new-widget state in this
} particular function.

Yes, that would work in _zle, but other callers of _arguments are already
passing a description which _arguments parses out, only to then throw it
away.  Shouldn't we keep that parsing in one place and make it useful?

Re: using the description from _arguments foo:description:->state

[Oliver Kiddle]

Bart wrote:
> On Sep 21,  1:39pm, Oliver Kiddle wrote:
> } While we could, there's always the possibility that some function
> } somewhere has a -d that's intended to me an argument to be completed.
> 
> That condition is already handled by using e.g. "_arguments -n : -n".

True but that doesn't help for backward compatibility with old
completion functions.

> Before producing my patch I first thought about digging around in $expl,
> but $expl is already marked up for use with _format, so that doesn't
> really work.  Also $expl is only passed *down* from _arguments, not back
> up to the caller.

Well states are just a lazy way of avoiding separate functions. It'd be
nicer to get $expl across to them despite it not strictly being down
the call stack. However, I'll admit that having the description alone
is better than nothing.

> Yes, that's what my patch does.  $state_descr is an array parallel to

Sorry, it looks like I should have checked and read the beginning of the
thread before weighing in.

> Yes, that would work in _zle, but other callers of _arguments are already
> passing a description which _arguments parses out, only to then throw it
> away.  Shouldn't we keep that parsing in one place and make it useful?

Yes, fair enough. In many cases, the descriptions have been left blank
where they get thrown out in this manner but it would be nicer to have
them cleanly in one place.

So given that you "don't think adding -d buys much" and there is the
compatibility issue I mentioned, what would you suggest?  A local in
_main_complete would fall down in the case of a function using states
calling another function that also uses states but which doesn't declare
state_descr local. _arguments isn't used much from helpers but _values
is. Perhaps _arguments/_values could leave state_descr alone where
the description is blank or consists of just a space. That way the
documentation could also indicate that functions need only declare
state_descr local if they give a description for their states. The
implementation would have to take care in the case where $state has more
than one element, however.

Oliver

Re: using the description from _arguments foo:description:->state

[Bart Schaefer]

On Sep 21,  7:10pm, Oliver Kiddle wrote:
}
} So given that you "don't think adding -d buys much" and there is the
} compatibility issue I mentioned, what would you suggest?  A local in
} _main_complete would fall down in the case of a function using states
} calling another function that also uses states but which doesn't declare
} state_descr local.

That'd be a bug in one or the other of those functions, would it not?
Declaring state_descr in _main_complete is just to keep it from leaking
out to the top level, not to make it possible for functions that need
it to avoid declaring it.  There's a huge list of other variables that
already "fall down" in this way; one more isn't going to kill us.  (I
don't *like* the necessity of that huge list, but there it is.)

} Perhaps _arguments/_values could leave state_descr alone where
} the description is blank or consists of just a space.

Unfortunately both _arguments and _values need to erase the existing
value of the state-related variables before they know whether there
are any states or descriptions to assign.  The right way to do this
probably would have been to pass the names of the variables to set
rather than rely entirely on scoping, but [unless someone wants to
undertake a major drudgework project to update the whole Completion
tree] it's too late to deal with that now.

E.g., _arguments has (after my not - committed patch):

  context=()
  state=()
  state_descr=()

These could all become

  (( $+context )) && context=()
  (( $+state )) && state=()
  (( $+state_descr )) && state_descr=()

and similarly for the place that useful values are assigned to each of
them (there's only one such place), but that still doesn't help for a
recursive call to _arguments or for a call to _values from _arguments.