PATCH v2 (complete): Implement zle -P

PATCH v2 (complete): Implement zle -P

[Mikael Magnusson]

---

On Fri, Dec 31, 2010 at 9:28 AM, Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Dec 30,  1:59pm, Wayne Davison wrote:
> }
> } I've coded up a patch that adds the -y option to zle so that I can define a
> } "zle -Ny yank" widget (as well as yank-pop), which marks it as being a
> } ZLE_YANK equivalent.
>
> This sort of follows from "zle -C".  For completion widgets, it was
> recognized that there are a set of behaviors which are the model for
> any newly-added widget, and the builtin completion widgets provide
> names for those behaviors.
>
> Perhaps a general solution would be that for *any* widget, not just
> a completion one, it should be possible to name a "prototype" widget
> whose behavior the new widget is intended to simulate or replace.
> "zle -P WIDGET PROTOTYPE-WIDGET [FUNCTION]" where "zle -N ..." becomes
> a special case equivalent to "zle -P WIDGET self-insert FUNCTION",
> or something to that effect (or maybe undefined-key instead).
>
> Then one could write, e.g.,
>
>   zle -P history-search-sideways history-incremental-search-forward
>
> and zle would "know" to invoke the minibuffer and re-call the function
> as each new keystroke is typed, without that needing to be coded as a
> loop in the widget itself.
>
> Then the question would be whether we still need "zle -C ..." other
> than for backward compatibility.
>
> The only other solution to this that I've thought of would be to follow
> the example of the auto-suffix-remove and auto-suffix-retain widgets,
> that is, have a special widget whose only effect is to have the side-
> effect of enabling yank-pop on the next interaction: yank-pop-enable
> perhaps (is yank-pop-disable ever needed?).

Here's a more complete patch for this. sorry about sending a bunch of
incremental patches, I seem to tend to do that.

On Mon, Aug 31, 2015 at 7:11 PM, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> It's not entirely clear what this does without documentation, but as the
> flags are a bit obcsure anyway, it might not be all that clear how to
> document it, either.

Indeed, I'm not sure if a user can figure out which widgets are useful
to override in this way.  Would it be more useful to just do what Wayne
did originally and let zle -N take some flags that specify which specific
flags the new widget should have after all? The advantage of this method
is that if we add more flags, users can wrap those widgets without us
remembering to add a new zle -N flag as well.

I replaced all of my zle -C with zle -P and my custom completion widgets
work the same, so that's a good sign at least.

The following are the usecases from the other mails to centralize.

I can now do this (which is why I originally found this old thread, if
anyone was curious):

function _start_paste() {
  local content
  local a b
  local oldcutbuf="$CUTBUFFER"
  # I haven't quite decided how I want to control quoting yet.
  [[ $_SPACE_AFTER_PASTE_QUOTE = 1 ]]; a=$?
  (( $+NUMERIC )) || [[ $KEYS = $'\e\e'* ]]; b=$?
  zle .$WIDGET -N content
  if [[ $a -ne $b ]]; then
    CUTBUFFER=${(q-)content}' '
  else
    CUTBUFFER=$content
  fi
  zle .yank
  CUTBUFFER="$oldcutbuf"
}
zle -P bracketed-paste bracketed-paste _start_paste

And my pastes are appropriately highlit. (Wouldn't it really be more
useful to have the middle argument optional? I see how it might be
confusing if you're used to the zle -N behavior though.)

% zle -P myyank yank myyank
% myyank () {
    zle -M hello
    local oldbuf=$CUTBUFFER CUTBUFFER=${(U)CUTBUFFER
    zle .yank
    CUTBUFFER=$oldbuf
}
  # put asdfdδ in the cutbuffer here
  # press yank and myyank alternatingly
% asdfdδASDFDΔASDFDΔASDFDΔasdfdδasdfdδasdfdδASDFDΔASDFDΔASDFDΔ
  # each word gets highlighted appropriately in both cases

 Src/Zle/zle_main.c   |  5 +++--
 Src/Zle/zle_thingy.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
index e610ae1..6df7367 100644
--- a/Src/Zle/zle_main.c
+++ b/Src/Zle/zle_main.c
@@ -1396,7 +1396,8 @@ execzlefunc(Thingy func, char **args, int set_bindk)
 	    opts[XTRACE] = oxt;
 	    sfcontext = osc;
 	    endparamscope();
-	    lastcmd = 0;
+	    if (!(w->flags & ZLE_NOTCOMMAND))
+		lastcmd = w->flags;
 	    r = 1;
 	    redup(osi, 0);
 	}
@@ -1975,7 +1976,7 @@ zle_main_entry(int cmd, va_list ap)
 static struct builtin bintab[] = {
     BUILTIN("bindkey", 0, bin_bindkey, 0, -1, 0, "evaM:ldDANmrsLRp", NULL),
     BUILTIN("vared",   0, bin_vared,   1,  1, 0, "aAcef:hi:M:m:p:r:t:", NULL),
-    BUILTIN("zle",     0, bin_zle,     0, -1, 0, "aAcCDFgGIKlLmMNrRTUw", NULL),
+    BUILTIN("zle",     0, bin_zle,     0, -1, 0, "aAcCDFgGIKlLmMNPrRTUw", NULL),
 };
 
 /* The order of the entries in this table has to match the *HOOK
diff --git a/Src/Zle/zle_thingy.c b/Src/Zle/zle_thingy.c
index 7fd3a59..20a1de9 100644
--- a/Src/Zle/zle_thingy.c
+++ b/Src/Zle/zle_thingy.c
@@ -347,6 +347,7 @@ bin_zle(char *name, char **args, Options ops, UNUSED(int func))
 	{ 'A', bin_zle_link, 2,  2 },
 	{ 'N', bin_zle_new,  1,  2 },
 	{ 'C', bin_zle_complete, 3, 3 },
+	{ 'P', bin_zle_prototype, 2, 3},
 	{ 'R', bin_zle_refresh, 0, -1 },
 	{ 'M', bin_zle_mesg, 1, 1 },
 	{ 'U', bin_zle_unget, 1, 1 },
@@ -591,6 +592,50 @@ bin_zle_new(char *name, char **args, UNUSED(Options ops), UNUSED(char func))
 
 /**/
 static int
+bin_zle_prototype(char *name, char **args, UNUSED(Options ops), UNUSED(char func))
+{
+    Thingy t;
+    Widget w, pw;
+    char *funcname = args[2] ? args[2] : args[0];
+
+    t = rthingy((args[1][0] == '.') ? args[1] : dyncat(".", args[1]));
+    pw = t->widget;
+    unrefthingy(t);
+
+    if (!pw) {
+	zwarnnam(name, "invalid widget `%s'", args[1]);
+	return 1;
+    }
+    if (pw->flags & ZLE_ISCOMP &&
+	require_module("zsh/complete", NULL) == 1)
+    {
+	zwarnnam(name, "can't load complete module");
+	return 1;
+    }
+    w = zalloc(sizeof(*w));
+    w->flags = pw->flags & ~(WIDGET_INT|ZLE_ISCOMP);
+    w->first = NULL;
+    if (pw->flags & ZLE_ISCOMP) {
+	w->flags |= WIDGET_NCOMP;
+	w->u.comp.fn = pw->u.fn;
+	w->u.comp.wid = ztrdup(args[1]);
+	w->u.comp.func = ztrdup(funcname);
+    } else {
+	w->u.fnnam = ztrdup(funcname);
+    }
+    if (bindwidget(w, rthingy(args[0]))) {
+	freewidget(w);
+	zwarnnam(name, "widget name `%s' is protected", args[0]);
+	return 1;
+    }
+    if (w->flags & WIDGET_NCOMP)
+	hascompwidgets++;
+
+    return 0;
+}
+
+/**/
+static int
 bin_zle_complete(char *name, char **args, UNUSED(Options ops), UNUSED(char func))
 {
     Thingy t;
diff --git i/Doc/Zsh/zle.yo w/Doc/Zsh/zle.yo
index c06e226..a8b7476 100644
--- i/Doc/Zsh/zle.yo
+++ w/Doc/Zsh/zle.yo
@@ -404,6 +404,7 @@ xitem(tt(zle) tt(-D) var(widget) ...)
 xitem(tt(zle) tt(-A) var(old-widget) var(new-widget))
 xitem(tt(zle) tt(-N) var(widget) [ var(function) ])
 xitem(tt(zle) tt(-C) var(widget) var(completion-widget) var(function))
+xitem(tt(zle) tt(-P) var(widget) var(prototype-widget) [ var(function) ])
 xitem(tt(zle) tt(-R) [ tt(-c) ] [ var(display-string) ] [ var(string) ... ])
 xitem(tt(zle) tt(-M) var(string))
 xitem(tt(zle) tt(-U) var(string))
@@ -474,6 +475,20 @@ ifzman(zmanref(zshcompwid))\
 ifnzman(noderef(Completion Widgets))\
 .
 )
+item(tt(-P) var(widget) var(prototype-widget) [ var(function) ])(
+Create a user-defined widget.  If there is already a widget with the
+specified name, it is overwritten.  When the new
+widget is invoked from within the editor, the specified shell var(function)
+is called.  If no function name is specified, it defaults to
+the same name as the widget.  The widget will behave as the given
+var(prototype-widget).  If it is a completion widget then this works like
+tt(zle -C) above.  An example of a useful case is wrapping the tt(yank)
+widget, without using tt(zle -P), the yanked text will not be highlighted.
+For further information, see
+ifzman(the section `Widgets' below)\
+ifnzman(noderef(Zle Widgets))\
+.
+)
 item(tt(-R) [ tt(-c) ] [ var(display-string) ] [ var(string) ... ])(
 Redisplay the command line; this is to be called from within a user-defined
 widget to allow changes to become visible.  If a var(display-string) is
@@ -986,18 +1001,19 @@ The name of the widget currently being executed; read-only.
 vindex(WIDGETFUNC)
 item(tt(WIDGETFUNC) (scalar))(
 The name of the shell function that implements a widget defined with
-either tt(zle -N) or tt(zle -C).  In the former case, this is the second
+either tt(zle -N), tt(zle -C) or tt(zle -P).  In the former case, this is the second
 argument to the tt(zle -N) command that defined the widget, or
-the first argument if there was no second argument.  In the latter case
-this is the third argument to the tt(zle -C) command that defined the
-widget.  Read-only.
+the first argument if there was no second argument.  For tt(zle -C)
+and tt(zle -P), this is the third argument to the tt(zle -C) command
+that defined the widget.  Read-only.
 )
 vindex(WIDGETSTYLE)
 item(tt(WIDGETSTYLE) (scalar))(
 Describes the implementation behind the completion widget currently being
-executed; the second argument that followed tt(zle -C) when the widget was
-defined.  This is the name of a builtin completion widget.  For widgets
-defined with tt(zle -N) this is set to the empty string.  Read-only.
+executed; the second argument that followed tt(zle -C) or tt(zle -P) when
+the widget was defined.  This is the name of a builtin completion widget.
+For widgets defined with tt(zle -N) this is set to the empty string.
+Read-only.
 )
 vindex(ZLE_STATE)
 item(tt(ZLE_STATE) (scalar))(

-- 
2.5.0

Re: PATCH v2 (complete): Implement zle -P

[Bart Schaefer]

I'd forgotten that I originally suggested "zle -P" ...

On Sep 1,  8:07am, Mikael Magnusson wrote:
}
} I can now do this (which is why I originally found this old thread, if
} anyone was curious):
} 
} function _start_paste() {
}   local content
}   local a b
}   local oldcutbuf="$CUTBUFFER"
}   # I haven't quite decided how I want to control quoting yet.
}   [[ $_SPACE_AFTER_PASTE_QUOTE = 1 ]]; a=$?
}   (( $+NUMERIC )) || [[ $KEYS = $'\e\e'* ]]; b=$?
}   zle .$WIDGET -N content
}   if [[ $a -ne $b ]]; then
}     CUTBUFFER=${(q-)content}' '
}   else
}     CUTBUFFER=$content
}   fi
}   zle .yank
}   CUTBUFFER="$oldcutbuf"
} }
} zle -P bracketed-paste bracketed-paste _start_paste

So I think this is:

autoload -Uz bracketed-paste-magic
zle -N bracketed-paste bracketed-paste-magic
zstyle :bracketed-paste-magic paste-finish quote-paste
zstyle -e :bracketed-paste-magic:finish quote-style \
    '() { local a b;
          [[ $_SPACE_AFTER_PASTE_QUOTE = 1 ]]; a=$?;
          (( $+NUMERIC )) || [[ $KEYS = $'\''\e\e'\''* ]]; b=$? ;
          [[ $a -ne $b ]] } && reply=(q-) || reply=(none)'

BTW what's the point of doing "zle .$WIDGET ..." when the only widget
this can possibly work with is .bracketed-paste ?

Re: PATCH v2 (complete): Implement zle -P

[Oliver Kiddle]

Mikael Magnusson wrote:
> 
> Indeed, I'm not sure if a user can figure out which widgets are useful
> to override in this way.  Would it be more useful to just do what Wayne
> did originally and let zle -N take some flags that specify which specific
> flags the new widget should have after all? The advantage of this method
> is that if we add more flags, users can wrap those widgets without us
> remembering to add a new zle -N flag as well.

I think that approach is better than the prototypes. Otherwise, users
will end up needing to dig into the source code to find arbitrary
widgets that happen to have the right combination of flags. It is a
lot easier to document a list of flags. It would also make it easier
to only offer a subset of the ZLE_ flags. Allowing a flag requires us
to do the work of fully checking the implications of making that flag
user-tweakable. It might also constrain us in the future as we preserve
backward compatibility if we don't limit those that are available.

If we do go down the route of allowing flags to be set for widgets, it
needn't be specific to user-defined widgets. I currently use the
following:
  vi-cmd-suffix-retain () {
    zle auto-suffix-retain
    zle vi-cmd-mode
  }

That might just be zle -f auto-suffix-retain vi-cmd-mode
That should not be possible for the dot variants - .vi-cmd-mode
The opposite might also be needed with +f or a no- prefix.
That could also lead to a way to make self-insert not clear the current
region by making that behaviour a flag. See 33626

As Bart mentions in 28560, the auto-suffix-retain precedent would
suggest that we should perhaps consider another yank-pop-enable widget
(or perhaps hold-yank-state given that this now affects highlighting and
not just yank-pop). An advantage of that approach over the flags is that
the full implementation of the widget is contained within the function
definition file. With the flags, we might end up needing something like
#compdef so you can put #zledef -f yank as the first line of the
function definition. Another possibility would be to try to make
something like hold-yank-state automatic - assuming that hypothetical
widget was called after every yank/yank-pop/vi-put-* in a user-defined
widget.

Either way, how can we be sure that yankb/yanke are even vaguely
sensible after the user-defined widget has finished?

Oliver

Re: PATCH v2 (complete): Implement zle -P

[Peter Stephenson]

On Tue, 1 Sep 2015 11:50:27 +0200
Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> Mikael Magnusson wrote:
> > Indeed, I'm not sure if a user can figure out which widgets are useful
> > to override in this way.  Would it be more useful to just do what Wayne
> > did originally and let zle -N take some flags that specify which specific
> > flags the new widget should have after all? The advantage of this method
> > is that if we add more flags, users can wrap those widgets without us
> > remembering to add a new zle -N flag as well.
> 
> I think that approach is better than the prototypes.

I'd be inclined to think this is both clearer and more powerful, too.

I think originally there was resistance to this on the basis that the
flags were a hack we shouldn't really have at all, let alone expose.
However, it's now obvious they're needed, and documenting them would
probably be a good thing anyway.  So exposing them for use becomes a
natural extension of that.

pws

Re: PATCH v2 (complete): Implement zle -P

[Bart Schaefer]

On Sep 1, 11:50am, Oliver Kiddle wrote:
} 
} As Bart mentions in 28560, the auto-suffix-retain precedent would
} suggest that we should perhaps consider another yank-pop-enable widget
} (or perhaps hold-yank-state given that this now affects highlighting and
} not just yank-pop). An advantage of that approach over the flags is that
} the full implementation of the widget is contained within the function
} definition file.

Another possibility is that "zle -f" or whatever letter we assign is
one of those special variants like "zle -R" / "zle -I" which only
have an effect when run from inside another widget.  This hypothetical
zle-switch could set the flags that are to be in effect at the time
the widget completes.

This would make "zle auto-suffix-retain" equivalent to (for example)
"zle -f auto-suffix-retain" and then we just add other strings for
the other flags, rather than adding more special widgets (which
clutter the namespace and can be uselessly overridden with "zle -N").

} With the flags, we might end up needing something like
} #compdef so you can put #zledef -f yank as the first line of the
} function definition.

Agreed that this is less good than being able to control the effect
from inside the widget implementation.

} Another possibility would be to try to make something like
} hold-yank-state automatic - assuming that hypothetical widget was
} called after every yank/yank-pop/vi-put-* in a user-defined widget.

I'm not sure I understand this one.  Do you mean that any time one
of the yank-related widgets is called, its flags would "stick" to the
calling widget (at least until there was another later yank call)?

Still, it'd be nice if one could do e.g.

    LBUFFER+=$SOMENEWTEXT
    zle -f yank

to indicate that the insertion into LBUFFER should be treated like a
yank even though it wasn't actually using the kill ring.  Maybe there
is no valid reason for that.

} Either way, how can we be sure that yankb/yanke are even vaguely
} sensible after the user-defined widget has finished?

Yes, that is an issue.  Probably I'd say than any future changes to
the content of BUFFER invalidate the whole yank state.  But that may
be an awful lot of new record-keeping.

PATCH: zle -f from inside widget to set flags

[Mikael Magnusson]

---

This works for "yank", if you inserted text with another yank widget.

It's not very likely to do anything useful for "keepsuffix", since the
code that handles that flag only triggers for internal widgets, so we need
to fiddle some more with that path. I don't really know what menucmp,
linemove and kill are for. The others seemed definitely not useful,
but I could be wrong.

Thoughts? Am I correct in assuming it's pretty safe to use bindk like that?

We could potentially pass yankb and yanke as arguments like zle -f
yank15:36 (but with some nicer syntax) to make it work in the general case™.

 Src/Zle/zle_main.c   |  5 +++--
 Src/Zle/zle_thingy.c | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
index 5440a48..af878eb 100644
--- a/Src/Zle/zle_main.c
+++ b/Src/Zle/zle_main.c
@@ -1396,7 +1396,8 @@ execzlefunc(Thingy func, char **args, int set_bindk)
 	    opts[XTRACE] = oxt;
 	    sfcontext = osc;
 	    endparamscope();
-	    lastcmd = 0;
+	    lastcmd = w->flags;
+	    w->flags = 0;
 	    r = 1;
 	    redup(osi, 0);
 	}
@@ -1975,7 +1976,7 @@ zle_main_entry(int cmd, va_list ap)
 static struct builtin bintab[] = {
     BUILTIN("bindkey", 0, bin_bindkey, 0, -1, 0, "evaM:ldDANmrsLRp", NULL),
     BUILTIN("vared",   0, bin_vared,   1,  1, 0, "aAcef:hi:M:m:p:r:t:", NULL),
-    BUILTIN("zle",     0, bin_zle,     0, -1, 0, "aAcCDFgGIKlLmMNrRTUw", NULL),
+    BUILTIN("zle",     0, bin_zle,     0, -1, 0, "aAcCDfFgGIKlLmMNrRTUw", NULL),
 };
 
 /* The order of the entries in this table has to match the *HOOK
diff --git a/Src/Zle/zle_thingy.c b/Src/Zle/zle_thingy.c
index 7fd3a59..8b7390c 100644
--- a/Src/Zle/zle_thingy.c
+++ b/Src/Zle/zle_thingy.c
@@ -352,6 +352,7 @@ bin_zle(char *name, char **args, Options ops, UNUSED(int func))
 	{ 'U', bin_zle_unget, 1, 1 },
 	{ 'K', bin_zle_keymap, 1, 1 },
 	{ 'I', bin_zle_invalidate, 0, 0 },
+	{ 'f', bin_zle_flags, 1, -1 },
 	{ 'F', bin_zle_fd, 0, 2 },
 	{ 'T', bin_zle_transform, 0, 2},
 	{ 0,   bin_zle_call, 0, -1 },
@@ -625,6 +626,38 @@ bin_zle_complete(char *name, char **args, UNUSED(Options ops), UNUSED(char func)
 
 /**/
 static int
+bin_zle_flags(char *name, char **args, UNUSED(Options ops), UNUSED(char func))
+{
+    char **flag;
+
+    if (!zle_usable()) {
+	zwarnnam(name, "can only set flags from a widget");
+	return 1;
+    }
+
+    if (bindk) {
+	Widget w = bindk->widget;
+	if (w) {
+	    for (flag = args; *flag; flag++) {
+		if (!strcmp(*flag, "yank"))
+		    w->flags |= ZLE_YANKBEFORE;
+		else if (!strcmp(*flag, "menucmp"))
+		    w->flags |= ZLE_MENUCMP;
+		else if (!strcmp(*flag, "linemove"))
+		    w->flags |= ZLE_LINEMOVE;
+		else if (!strcmp(*flag, "kill"))
+		    w->flags |= ZLE_KILL;
+		else if (!strcmp(*flag, "keepsuffix"))
+		    w->flags |= ZLE_KEEPSUFFIX;
+		else
+		    zwarnnam(name, "invalid flag `%s' given to zle -f", *flag);
+	    }
+	}
+    }
+}
+
+/**/
+static int
 zle_usable()
 {
     return zleactive && !incompctlfunc && !incompfunc
-- 
2.5.0

Re: PATCH: zle -f from inside widget to set flags

[Mikael Magnusson]

On Thu, Sep 3, 2015 at 12:38 PM, Mikael Magnusson <mikachu@gmail.com> wrote:
> This works for "yank", if you inserted text with another yank widget.
>
> It's not very likely to do anything useful for "keepsuffix", since the
> code that handles that flag only triggers for internal widgets, so we need
> to fiddle some more with that path. I don't really know what menucmp,
> linemove and kill are for. The others seemed definitely not useful,
> but I could be wrong.
>
> Thoughts? Am I correct in assuming it's pretty safe to use bindk like that?
>
> We could potentially pass yankb and yanke as arguments like zle -f
> yank15:36 (but with some nicer syntax) to make it work in the general case™.

I haven't tried, but I highly suspect that if you call another user
widget, and that widget uses zle -f, then this won't have any effect since
the parent widget's flags will trump those. That's probably okay. If not,
we could add a 'zle -f copy' that sticks lastcmd in w->flags (not tested).

>  Src/Zle/zle_main.c   |  5 +++--
>  Src/Zle/zle_thingy.c | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
> index 5440a48..af878eb 100644
> --- a/Src/Zle/zle_main.c
> +++ b/Src/Zle/zle_main.c
> @@ -1396,7 +1396,8 @@ execzlefunc(Thingy func, char **args, int set_bindk)
>             opts[XTRACE] = oxt;
>             sfcontext = osc;
>             endparamscope();
> -           lastcmd = 0;
> +           lastcmd = w->flags;
> +           w->flags = 0;
>             r = 1;
>             redup(osi, 0);
>         }
> @@ -1975,7 +1976,7 @@ zle_main_entry(int cmd, va_list ap)
>  static struct builtin bintab[] = {
>      BUILTIN("bindkey", 0, bin_bindkey, 0, -1, 0, "evaM:ldDANmrsLRp", NULL),
>      BUILTIN("vared",   0, bin_vared,   1,  1, 0, "aAcef:hi:M:m:p:r:t:", NULL),
> -    BUILTIN("zle",     0, bin_zle,     0, -1, 0, "aAcCDFgGIKlLmMNrRTUw", NULL),
> +    BUILTIN("zle",     0, bin_zle,     0, -1, 0, "aAcCDfFgGIKlLmMNrRTUw", NULL),
>  };
>
>  /* The order of the entries in this table has to match the *HOOK
> diff --git a/Src/Zle/zle_thingy.c b/Src/Zle/zle_thingy.c
> index 7fd3a59..8b7390c 100644
> --- a/Src/Zle/zle_thingy.c
> +++ b/Src/Zle/zle_thingy.c
> @@ -352,6 +352,7 @@ bin_zle(char *name, char **args, Options ops, UNUSED(int func))
>         { 'U', bin_zle_unget, 1, 1 },
>         { 'K', bin_zle_keymap, 1, 1 },
>         { 'I', bin_zle_invalidate, 0, 0 },
> +       { 'f', bin_zle_flags, 1, -1 },
>         { 'F', bin_zle_fd, 0, 2 },
>         { 'T', bin_zle_transform, 0, 2},
>         { 0,   bin_zle_call, 0, -1 },
> @@ -625,6 +626,38 @@ bin_zle_complete(char *name, char **args, UNUSED(Options ops), UNUSED(char func)
>
>  /**/
>  static int
> +bin_zle_flags(char *name, char **args, UNUSED(Options ops), UNUSED(char func))
> +{
> +    char **flag;
> +
> +    if (!zle_usable()) {
> +       zwarnnam(name, "can only set flags from a widget");
> +       return 1;
> +    }
> +
> +    if (bindk) {
> +       Widget w = bindk->widget;
> +       if (w) {
> +           for (flag = args; *flag; flag++) {
> +               if (!strcmp(*flag, "yank"))
> +                   w->flags |= ZLE_YANKBEFORE;
> +               else if (!strcmp(*flag, "menucmp"))
> +                   w->flags |= ZLE_MENUCMP;
> +               else if (!strcmp(*flag, "linemove"))
> +                   w->flags |= ZLE_LINEMOVE;
> +               else if (!strcmp(*flag, "kill"))
> +                   w->flags |= ZLE_KILL;
> +               else if (!strcmp(*flag, "keepsuffix"))
> +                   w->flags |= ZLE_KEEPSUFFIX;
> +               else
> +                   zwarnnam(name, "invalid flag `%s' given to zle -f", *flag);
> +           }
> +       }
> +    }
> +}
> +
> +/**/
> +static int
>  zle_usable()
>  {
>      return zleactive && !incompctlfunc && !incompfunc
> --
> 2.5.0
>



-- 
Mikael Magnusson

Re: PATCH v2 (complete): Implement zle -P

[Daniel Shahaf]

Peter Stephenson wrote on Tue, Sep 01, 2015 at 11:03:31 +0100:
> On Tue, 1 Sep 2015 11:50:27 +0200
> Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> > Mikael Magnusson wrote:
> > > Indeed, I'm not sure if a user can figure out which widgets are useful
> > > to override in this way.  Would it be more useful to just do what Wayne
> > > did originally and let zle -N take some flags that specify which specific
> > > flags the new widget should have after all? The advantage of this method
> > > is that if we add more flags, users can wrap those widgets without us
> > > remembering to add a new zle -N flag as well.
> > 
> > I think that approach is better than the prototypes.
> 
> I'd be inclined to think this is both clearer and more powerful, too.

The 'zle -P' form is nice when wrapping arbitrary widgets and wanting to
preserve their properties.  For example:

    zsh -f
    bindkey -e
    bkw-wrapper() { zle .backward-kill-word }
    zle -N backward-kill-word bkw-wrapper
    echo foo bar<^W><^W><^Y>

This results in "echo foo <CURSOR>", but in "echo foo bar <CURSOR>"
without the third line — because 'bkw-wrapper' lacks the ZLE_KILL flag
which backward-kill-word has.

When I wrap a built-in widget, I would rather just say "Make my widget
like that other widget" than enumerate all flags my widget requires;
but when I define a new widget from scratch, being able to enumerate all
flags my widgets required would be good.

So, in summary, I think both forms are useful: 'zle -P' is useful for
wrapping widgets and 'zle -f' for defining new ones.

Cheers,

Daniel

P.S. As a concrete example, in zsh-syntax-highlighting, the difference
in flags between a built-in widget and a user-defined wrapper thereof
leads to issues such as [1], where wrapping a widget causes it to change
behaviour: https://github.com/zsh-users/zsh-syntax-highlighting/issues/150
I believe using 'zle -P' to define the wrapper widget would immediately
solve that issue.

Re: PATCH v2 (complete): Implement zle -P

[Mikael Magnusson]

On Sun, Sep 6, 2015 at 11:50 AM, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Peter Stephenson wrote on Tue, Sep 01, 2015 at 11:03:31 +0100:
>> On Tue, 1 Sep 2015 11:50:27 +0200
>> Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
>> > Mikael Magnusson wrote:
>> > > Indeed, I'm not sure if a user can figure out which widgets are useful
>> > > to override in this way.  Would it be more useful to just do what Wayne
>> > > did originally and let zle -N take some flags that specify which specific
>> > > flags the new widget should have after all? The advantage of this method
>> > > is that if we add more flags, users can wrap those widgets without us
>> > > remembering to add a new zle -N flag as well.
>> >
>> > I think that approach is better than the prototypes.
>>
>> I'd be inclined to think this is both clearer and more powerful, too.
>
> The 'zle -P' form is nice when wrapping arbitrary widgets and wanting to
> preserve their properties.  For example:
>
>     zsh -f
>     bindkey -e
>     bkw-wrapper() { zle .backward-kill-word }
>     zle -N backward-kill-word bkw-wrapper
>     echo foo bar<^W><^W><^Y>
>
> This results in "echo foo <CURSOR>", but in "echo foo bar <CURSOR>"
> without the third line — because 'bkw-wrapper' lacks the ZLE_KILL flag
> which backward-kill-word has.
>
> When I wrap a built-in widget, I would rather just say "Make my widget
> like that other widget" than enumerate all flags my widget requires;
> but when I define a new widget from scratch, being able to enumerate all
> flags my widgets required would be good.
>
> So, in summary, I think both forms are useful: 'zle -P' is useful for
> wrapping widgets and 'zle -f' for defining new ones.
>
> Cheers,
>
> Daniel
>
> P.S. As a concrete example, in zsh-syntax-highlighting, the difference
> in flags between a built-in widget and a user-defined wrapper thereof
> leads to issues such as [1], where wrapping a widget causes it to change
> behaviour: https://github.com/zsh-users/zsh-syntax-highlighting/issues/150
> I believe using 'zle -P' to define the wrapper widget would immediately
> solve that issue.

You can believe it, but it's not the case. The code that handles most
of these flags first checks that the widget is internal, and not a
shell wrapper function. Some more changes would be needed to make this
work. The yank flag is the only one that does something useful at the
moment.

-- 
Mikael Magnusson