zsh-workers
 help / color / mirror / code / Atom feed
* Proposal: Let compinit make standard widgets with _generic instead of _main_complete
@ 2021-03-19 22:23 Marlon Richert
  2021-03-20 18:09 ` Bart Schaefer
  0 siblings, 1 reply; 13+ messages in thread
From: Marlon Richert @ 2021-03-19 22:23 UTC (permalink / raw)
  To: Zsh hackers list

How would you feel about changing compinit, so that it rebinds the
standard widgets with `zle -C $_i_line .$_i_line _generic` instead of
`zle -C $_i_line .$_i_line _main_complete`?
(https://github.com/zsh-users/zsh/blob/f4a248f9d38dc02d65610395f4c7f9a95a5d6612/Completion/compinit#L558)

This would make it easier for the end user to define, for example,
different `menu` styles for different widgets. Without using
`_generic`, the end user cannot out-of-the-box define different styles
for different widgets.

Here's the patch:

diff --git Completion/compinit Completion/compinit
index e81cd1604..212bc7cf3 100644
--- Completion/compinit
+++ Completion/compinit
@@ -555,9 +555,9 @@ fi
 for _i_line in complete-word delete-char-or-list expand-or-complete \
   expand-or-complete-prefix list-choices menu-complete \
   menu-expand-or-complete reverse-menu-complete; do
-  zle -C $_i_line .$_i_line _main_complete
+  zle -C $_i_line .$_i_line _generic
 done
-zle -la menu-select && zle -C menu-select .menu-select _main_complete
+zle -la menu-select && zle -C menu-select .menu-select _generic

 # If the default completer set includes _expand, and tab is bound
 # to expand-or-complete, rebind it to complete-word instead.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Proposal: Let compinit make standard widgets with _generic instead of _main_complete
  2021-03-19 22:23 Proposal: Let compinit make standard widgets with _generic instead of _main_complete Marlon Richert
@ 2021-03-20 18:09 ` Bart Schaefer
  2021-03-20 23:52   ` Bart Schaefer
  0 siblings, 1 reply; 13+ messages in thread
From: Bart Schaefer @ 2021-03-20 18:09 UTC (permalink / raw)
  To: Marlon Richert; +Cc: Zsh hackers list

On Fri, Mar 19, 2021 at 3:24 PM Marlon Richert <marlon.richert@gmail.com> wrote:
>
> How would you feel about changing compinit, so that it rebinds the
> standard widgets with `zle -C $_i_line .$_i_line _generic` instead of
> `zle -C $_i_line .$_i_line _main_complete`?

A potential issue with this is that it introduces a difference for
other wrapper widgets that assume _main_complete is THE completion
entry point.  For example, _complete_debug will no longer have an
accurate context as compared to complete-word.  That potentially
affects the following other functions:

Completion/Base/Completer/_expand_alias
Completion/Base/Widget/_bash_completions
Completion/Base/Widget/_complete_debug
Completion/Base/Widget/_complete_help
Completion/Base/Widget/_complete_tag
Completion/Base/Widget/_correct_word
Completion/Base/Widget/_expand_word
Completion/Base/Widget/_history_complete_word
Completion/Base/Widget/_next_tags

The ones of most concern are _complete_{debug,help,tag} (it's not
sufficient to simply call _generic inside those functions, because
_generic will insert the name of the wrapper widget into the context)
and of least concern is _bash_completions (which can presumably just
ignore this).  I haven't investigated whether any adjustment to the
others is possible or needed.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Proposal: Let compinit make standard widgets with _generic instead of _main_complete
  2021-03-20 18:09 ` Bart Schaefer
@ 2021-03-20 23:52   ` Bart Schaefer
  2021-03-21 15:19     ` Marlon Richert
  0 siblings, 1 reply; 13+ messages in thread
From: Bart Schaefer @ 2021-03-20 23:52 UTC (permalink / raw)
  To: Marlon Richert; +Cc: Zsh hackers list

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

On Sat, Mar 20, 2021 at 11:09 AM Bart Schaefer <schaefer@brasslantern.com>
wrote:

>
> A potential issue with this is that it introduces a difference for
> other wrapper widgets that assume _main_complete is THE completion
> entry point.
>

How about this?  It introduces a generic-widgets style, a list of widget
names that _main_complete hands off to _generic (which changes the context
and hands them back).

diff --git a/Completion/Base/Core/_main_complete
b/Completion/Base/Core/_main_complete
index 169ca1f40..62d94e3ae 100644
--- a/Completion/Base/Core/_main_complete
+++ b/Completion/Base/Core/_main_complete
@@ -10,6 +10,14 @@
 # In case non-standard separators are in use.
 local IFS=$' \t\n\0'

+# Test generic-widgets only in global context to avoid recursion
+if [[ "${curcontext:-:::}" == ::: ]] &&
+   zstyle -t ":completion:::::" generic-widgets "$WIDGET"
+then
+    _generic "$@"
+    return
+fi
+
 # If you want to complete only set or unset options for the unsetopt
 # and setopt builtin, un-comment these lines:
 #

[-- Attachment #2: Type: text/html, Size: 1488 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Proposal: Let compinit make standard widgets with _generic instead of _main_complete
  2021-03-20 23:52   ` Bart Schaefer
@ 2021-03-21 15:19     ` Marlon Richert
  2021-03-22  1:46       ` Bart Schaefer
  0 siblings, 1 reply; 13+ messages in thread
From: Marlon Richert @ 2021-03-21 15:19 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

On Sun, Mar 21, 2021 at 1:53 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
> +   zstyle -t ":completion:::::" generic-widgets "$WIDGET"

Feels weird to call that style `generic-widgets`, because it actually
makes the completion context less generic and more specific. IMO,
`specific-widgets` would be a more accurate name for this. Or perhaps
something like `(auto-)named-widgets`, borrowing from a sentence in
the manual:

> The _function,_ if completion is called from a named widget rather than through the normal completion system.

Yes, I think `_generic` is a pretty strange name for that function in
the first place. :)


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Proposal: Let compinit make standard widgets with _generic instead of _main_complete
  2021-03-21 15:19     ` Marlon Richert
@ 2021-03-22  1:46       ` Bart Schaefer
  2021-03-22  7:16         ` Marlon Richert
  0 siblings, 1 reply; 13+ messages in thread
From: Bart Schaefer @ 2021-03-22  1:46 UTC (permalink / raw)
  To: Marlon Richert; +Cc: Zsh hackers list

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

On Sun, Mar 21, 2021 at 8:20 AM Marlon Richert <marlon.richert@gmail.com>
wrote:

> On Sun, Mar 21, 2021 at 1:53 AM Bart Schaefer <schaefer@brasslantern.com>
> wrote:
> > +   zstyle -t ":completion:::::" generic-widgets "$WIDGET"
>
> Feels weird to call that style `generic-widgets`, because it actually
> makes the completion context less generic and more specific.


It refers to the completion results being generic, not to the context ...
but of course the style can be called anything, it's more the functionality
that's in question.

[-- Attachment #2: Type: text/html, Size: 939 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Proposal: Let compinit make standard widgets with _generic instead of _main_complete
  2021-03-22  1:46       ` Bart Schaefer
@ 2021-03-22  7:16         ` Marlon Richert
  2021-04-03 19:37           ` Lawrence Velázquez
  2021-04-04 18:35           ` Bart Schaefer
  0 siblings, 2 replies; 13+ messages in thread
From: Marlon Richert @ 2021-03-22  7:16 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

On Mon, Mar 22, 2021 at 3:46 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
> It refers to the completion results being generic, not to the context ... but of course the style can be called anything, it's more the functionality that's in question.

On Sun, Mar 21, 2021 at 1:53 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
> +if [[ "${curcontext:-:::}" == ::: ]] &&
> +   zstyle -t ":completion:::::" generic-widgets "$WIDGET"
> +then
> +    _generic "$@"
> +    return
> +fi

This feels like a rather roundabout solution. _generic itself already
checks whether $curcontext is set and then calls _main_complete. Why
not just do the straightforward thing, and let the widget call
_generic directly, which in turn will call _main_complete anyway?


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Proposal: Let compinit make standard widgets with _generic instead of _main_complete
  2021-03-22  7:16         ` Marlon Richert
@ 2021-04-03 19:37           ` Lawrence Velázquez
  2021-04-04 18:35           ` Bart Schaefer
  1 sibling, 0 replies; 13+ messages in thread
From: Lawrence Velázquez @ 2021-04-03 19:37 UTC (permalink / raw)
  To: zsh-workers

On Mon, Mar 22, 2021, at 3:16 AM, Marlon Richert wrote:
> On Mon, Mar 22, 2021 at 3:46 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
> > It refers to the completion results being generic, not to the context ... but of course the style can be called anything, it's more the functionality that's in question.
> 
> On Sun, Mar 21, 2021 at 1:53 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
> > +if [[ "${curcontext:-:::}" == ::: ]] &&
> > +   zstyle -t ":completion:::::" generic-widgets "$WIDGET"
> > +then
> > +    _generic "$@"
> > +    return
> > +fi
> 
> This feels like a rather roundabout solution. _generic itself already
> checks whether $curcontext is set and then calls _main_complete. Why
> not just do the straightforward thing, and let the widget call
> _generic directly, which in turn will call _main_complete anyway?

Anything else on this?

vq


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Proposal: Let compinit make standard widgets with _generic instead of _main_complete
  2021-03-22  7:16         ` Marlon Richert
  2021-04-03 19:37           ` Lawrence Velázquez
@ 2021-04-04 18:35           ` Bart Schaefer
  2021-04-04 19:31             ` Bart Schaefer
  2021-04-09 19:11             ` Oliver Kiddle
  1 sibling, 2 replies; 13+ messages in thread
From: Bart Schaefer @ 2021-04-04 18:35 UTC (permalink / raw)
  To: Marlon Richert; +Cc: Zsh hackers list

On Mon, Mar 22, 2021 at 12:17 AM Marlon Richert
<marlon.richert@gmail.com> wrote:
>
> On Sun, Mar 21, 2021 at 1:53 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
> > +if [[ "${curcontext:-:::}" == ::: ]] &&
> > +   zstyle -t ":completion:::::" generic-widgets "$WIDGET"
> > +then
> > +    _generic "$@"
> > +    return
> > +fi
>
> This feels like a rather roundabout solution. _generic itself already
> checks whether $curcontext is set and then calls _main_complete. Why
> not just do the straightforward thing, and let the widget call
> _generic directly, which in turn will call _main_complete anyway?

This is really only the first in a few steps of patch that I didn't
want to spend time on if this were rejected out of hand.

The problem with calling _generic at all (right now) is that it messes
with $curcontext in a way that may break other functions that rely on
_main_complete behavior.  If we can invert the call sequence so
_main_complete remains first, then _generic itself can be tweaked so
that it does NOT mung the context in that circumstance, while still
leaving the original semantics of creating a new widget with _generic
unchanged.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Proposal: Let compinit make standard widgets with _generic instead of _main_complete
  2021-04-04 18:35           ` Bart Schaefer
@ 2021-04-04 19:31             ` Bart Schaefer
  2021-04-09 19:11             ` Oliver Kiddle
  1 sibling, 0 replies; 13+ messages in thread
From: Bart Schaefer @ 2021-04-04 19:31 UTC (permalink / raw)
  To: Marlon Richert; +Cc: Zsh hackers list

On Sun, Apr 4, 2021 at 11:35 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> The problem with calling _generic at all (right now) is that it messes
> with $curcontext in a way that may break other functions that rely on
> _main_complete behavior.  If we can invert the call sequence so
> _main_complete remains first, then _generic itself can be tweaked so
> that it does NOT mung the context in that circumstance

Sorry, that's also an incomplete explanation, and it might not be only
_generic that needs tweaking.

Suppose I want to invoke _complete_debug.  It's supposed to emulate
complete-word, but has a hardwired call to _main_complete.  If
complete-word is _generic, then when I try _complete_debug I won't get
a backtrace that actually matches what happens when I complete-word.
If instead _main_complete runs _generic, the backtrace from
_complete_debug is correct, and there's an opportunity to build some
additional intelligence into _complete_debug because it can check
which widgets are _generic.  Similar things apply to _complete_help,
etc.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Proposal: Let compinit make standard widgets with _generic instead of _main_complete
  2021-04-04 18:35           ` Bart Schaefer
  2021-04-04 19:31             ` Bart Schaefer
@ 2021-04-09 19:11             ` Oliver Kiddle
  2021-04-09 19:35               ` Bart Schaefer
  1 sibling, 1 reply; 13+ messages in thread
From: Oliver Kiddle @ 2021-04-09 19:11 UTC (permalink / raw)
  To: Zsh hackers list

On 4 Apr, Bart Schaefer wrote:
> On Mon, Mar 22, 2021 at 12:17 AM Marlon Richert
> > > +if [[ "${curcontext:-:::}" == ::: ]] &&
> > > +   zstyle -t ":completion:::::" generic-widgets "$WIDGET"
> > > +then
> > > +    _generic "$@"
> > > +    return
> > > +fi
> >
> > This feels like a rather roundabout solution. _generic itself already

That does indeed seem rather roundabout. And all it achieves is a
different way to configure what can already be configured.

> > checks whether $curcontext is set and then calls _main_complete. Why
> > not just do the straightforward thing, and let the widget call
> > _generic directly, which in turn will call _main_complete anyway?
>
> This is really only the first in a few steps of patch that I didn't
> want to spend time on if this were rejected out of hand.
> The problem with calling _generic at all (right now) is that it messes
> with $curcontext in a way that may break other functions that rely on
> _main_complete behavior.  If we can invert the call sequence so
> _main_complete remains first, then _generic itself can be tweaked so
> that it does NOT mung the context in that circumstance, while still
> leaving the original semantics of creating a new widget with _generic
> unchanged.

The only thing _generic really does is shove $WIDGET into the second
field of $curcontext where _main_complete left this empty. I wouldn't
know why it was empty by default other than perhaps brevity. But it does
provide for a notion of a default behaviour for the tab key and widgets
like _complete_debug that want to follow that default. Certainly, I have
quite a few styles defined with the two consecutive colons hardcoded. I
can't see how we can change it in a backward compatible way even if we
determine that it would be better.

The intended way to do this is to use zle -C to define a custom widget
based on an existing one and _generic. And if you really want the
existing widget, zstyle -e allows you to check $WIDGET. Or is the aim
here actually something else like making it easier for a plugin to base
changes on underlying user settings.

Oliver


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Proposal: Let compinit make standard widgets with _generic instead of _main_complete
  2021-04-09 19:11             ` Oliver Kiddle
@ 2021-04-09 19:35               ` Bart Schaefer
  2021-05-09 20:51                 ` Lawrence Velázquez
  0 siblings, 1 reply; 13+ messages in thread
From: Bart Schaefer @ 2021-04-09 19:35 UTC (permalink / raw)
  To: Oliver Kiddle; +Cc: Zsh hackers list

On Fri, Apr 9, 2021 at 12:11 PM Oliver Kiddle <opk@zsh.org> wrote:
>
> The intended way to do this is to use zle -C to define a custom widget
> based on an existing one and _generic. And if you really want the
> existing widget, zstyle -e allows you to check $WIDGET. Or is the aim
> here actually something else like making it easier for a plugin to base
> changes on underlying user settings.

I think use of "zstyle -e" was either overlooked or was itself
considered too roundabout.

In another thread (zsh-newuser-install) the aim is to change the
behavior of the existing widget (to invoke menu selection) without
having to rebind the keys that reference it.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Proposal: Let compinit make standard widgets with _generic instead of _main_complete
  2021-04-09 19:35               ` Bart Schaefer
@ 2021-05-09 20:51                 ` Lawrence Velázquez
  2021-05-10 17:11                   ` Bart Schaefer
  0 siblings, 1 reply; 13+ messages in thread
From: Lawrence Velázquez @ 2021-05-09 20:51 UTC (permalink / raw)
  To: zsh-workers

On Fri, Apr 9, 2021, at 3:35 PM, Bart Schaefer wrote:
> On Fri, Apr 9, 2021 at 12:11 PM Oliver Kiddle <opk@zsh.org> wrote:
> >
> > The intended way to do this is to use zle -C to define a custom widget
> > based on an existing one and _generic. And if you really want the
> > existing widget, zstyle -e allows you to check $WIDGET. Or is the aim
> > here actually something else like making it easier for a plugin to base
> > changes on underlying user settings.
> 
> I think use of "zstyle -e" was either overlooked or was itself
> considered too roundabout.
> 
> In another thread (zsh-newuser-install) the aim is to change the
> behavior of the existing widget (to invoke menu selection) without
> having to rebind the keys that reference it.

Anything else on this?

vq


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Proposal: Let compinit make standard widgets with _generic instead of _main_complete
  2021-05-09 20:51                 ` Lawrence Velázquez
@ 2021-05-10 17:11                   ` Bart Schaefer
  0 siblings, 0 replies; 13+ messages in thread
From: Bart Schaefer @ 2021-05-10 17:11 UTC (permalink / raw)
  To: Zsh hackers list

On Sun, May 9, 2021 at 1:51 PM Lawrence Velázquez <larryv@zsh.org> wrote:
>
> Anything else on this?

Attempting to summarize the conclusions (so far):

There's no backward-compatible change that can be made to the widget
definitions.

_generic was not meant to be used to re-implement the default widgets;
the expected approach is to create a custom widget with a different
name.

It's possible to achieve the desired effect by using "zstyle -e" to
examine $WIDGET.  An example appeared in another thread:
  zstyle -e ':completion::*:default' menu \
    '[[ $WIDGET = (|reverse-)menu-complete ]] &&
    reply=(yes select interactive)'

I think it's therefore safe to say that we're not going to apply
either of the patches that have appeared earlier in this thread.

On a bit of further reflection, I think there's an even simpler
formulation that should give the same effect (excuse possible gmail
line wrap):
  zstyle -e ':completion::*:default' menu \
   'zstyle -a ":completion:${WIDGET}:${curcontext#*:}:default" menu reply'

That avoids the issue mentioned in workers/48397 wherein _generic
changes the call tree inside _complete_debug, but to completely mask
the effect one would have to use ${WIDGET#_complete_(debug|help)} or
similar.


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-05-10 17:12 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-19 22:23 Proposal: Let compinit make standard widgets with _generic instead of _main_complete Marlon Richert
2021-03-20 18:09 ` Bart Schaefer
2021-03-20 23:52   ` Bart Schaefer
2021-03-21 15:19     ` Marlon Richert
2021-03-22  1:46       ` Bart Schaefer
2021-03-22  7:16         ` Marlon Richert
2021-04-03 19:37           ` Lawrence Velázquez
2021-04-04 18:35           ` Bart Schaefer
2021-04-04 19:31             ` Bart Schaefer
2021-04-09 19:11             ` Oliver Kiddle
2021-04-09 19:35               ` Bart Schaefer
2021-05-09 20:51                 ` Lawrence Velázquez
2021-05-10 17:11                   ` 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).