* using the description from _arguments foo:description:->state @ 2011-09-11 15:04 Mikael Magnusson 2011-09-11 18:10 ` Bart Schaefer 0 siblings, 1 reply; 7+ messages in thread From: Mikael Magnusson @ 2011-09-11 15:04 UTC (permalink / raw) To: zsh workers 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: using the description from _arguments foo:description:->state 2011-09-11 15:04 using the description from _arguments foo:description:->state Mikael Magnusson @ 2011-09-11 18:10 ` Bart Schaefer 2011-09-20 15:16 ` Mikael Magnusson 0 siblings, 1 reply; 7+ messages in thread From: Bart Schaefer @ 2011-09-11 18:10 UTC (permalink / raw) To: zsh workers 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: using the description from _arguments foo:description:->state 2011-09-11 18:10 ` Bart Schaefer @ 2011-09-20 15:16 ` Mikael Magnusson 2011-09-21 11:39 ` Oliver Kiddle 0 siblings, 1 reply; 7+ messages in thread From: Mikael Magnusson @ 2011-09-20 15:16 UTC (permalink / raw) To: Bart Schaefer; +Cc: zsh workers 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: using the description from _arguments foo:description:->state 2011-09-20 15:16 ` Mikael Magnusson @ 2011-09-21 11:39 ` Oliver Kiddle 2011-09-21 15:50 ` Bart Schaefer 0 siblings, 1 reply; 7+ messages in thread From: Oliver Kiddle @ 2011-09-21 11:39 UTC (permalink / raw) To: zsh workers 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: using the description from _arguments foo:description:->state 2011-09-21 11:39 ` Oliver Kiddle @ 2011-09-21 15:50 ` Bart Schaefer 2011-09-21 17:10 ` Oliver Kiddle 0 siblings, 1 reply; 7+ messages in thread From: Bart Schaefer @ 2011-09-21 15:50 UTC (permalink / raw) To: zsh workers 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? ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: using the description from _arguments foo:description:->state 2011-09-21 15:50 ` Bart Schaefer @ 2011-09-21 17:10 ` Oliver Kiddle 2011-09-22 4:04 ` Bart Schaefer 0 siblings, 1 reply; 7+ messages in thread From: Oliver Kiddle @ 2011-09-21 17:10 UTC (permalink / raw) To: zsh workers 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: using the description from _arguments foo:description:->state 2011-09-21 17:10 ` Oliver Kiddle @ 2011-09-22 4:04 ` Bart Schaefer 0 siblings, 0 replies; 7+ messages in thread From: Bart Schaefer @ 2011-09-22 4:04 UTC (permalink / raw) To: zsh workers 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-09-22 4:04 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-09-11 15:04 using the description from _arguments foo:description:->state Mikael Magnusson 2011-09-11 18:10 ` Bart Schaefer 2011-09-20 15:16 ` Mikael Magnusson 2011-09-21 11:39 ` Oliver Kiddle 2011-09-21 15:50 ` Bart Schaefer 2011-09-21 17:10 ` Oliver Kiddle 2011-09-22 4:04 ` Bart Schaefer
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).