zsh-workers
 help / color / mirror / code / Atom feed
From: Daniel Shahaf <d.s@daniel.shahaf.name>
To: Mikael Magnusson <mikachu@gmail.com>
Cc: zsh-workers@zsh.org
Subject: Re: add-zle-hook-widget and multiple hooks
Date: Wed, 10 Jun 2020 13:34:52 +0000	[thread overview]
Message-ID: <20200610133452.025cfc54@tarpaulin.shahaf.local2> (raw)
In-Reply-To: <CAHYJk3Q+hiUAGdhwkP9GcZTYXJzEe0kjX3axZQ4to4-fY7oRxA@mail.gmail.com>

Mikael Magnusson wrote on Mon, 08 Jun 2020 19:52 +0200:
> On 6/8/20, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > [moving Eric to bcc]
> >
> > Mikael Magnusson wrote on Sat, 06 Jun 2020 13:58 +0200:  
> >> On 6/6/20, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:  
> >> > When two or more zle-line-pre-redraw hooks are registered using
> >> > add-zle-hook-widget, the value of $LASTWIDGET when the each hook is
> >> > called is the name of the former hook:
> >> >
> >> >     [Eric Freese wrote in
> >> > https://github.com/zsh-users/zsh-autosuggestions/issues/529#issuecomment-632113840]
> >> >     $ zsh -df
> >> >     % autoload add-zle-hook-widget
> >> >     % f() {}
> >> >     % g() { zle -M "$(typeset -p LASTWIDGET)" }
> >> >     % add-zle-hook-widget line-pre-redraw f
> >> >     % add-zle-hook-widget line-pre-redraw g
> >> >     % x<CURSOR>
> >> >     typeset -r LASTWIDGET=f
> >> >
> >> > The issue here is that g would like to to know what widget was invoked
> >> > immediately before the redraw.  In the example, that'd be self-insert.
> >> >
> >> > I've attached two proofs of concept.  WDYT?
> >> >
> >> > I'll add docs, etc, once an approach is chosen.
> >> >
> >> > Cheers,
> >> >
> >> > Daniel
> >> >
> >> > P.S.  For the latter patch, note that «zle $widget -f» is distinct from
> >> > «zle -f».  
> >>
> >> I think the warning message when -f is not followed by nolast should
> >> be phrased in the same way it would if there were other valid flags,
> >> since it would have to change when more are added anyway (we will
> >> probably not want to enumerate all possible flags in this warning
> >> message).  
> >
> > Thanks for the review.
> >
> > If we add more flags that are too many to list, then we should change
> > the error message, yes; nevertheless, _right now_ only one flag is
> > supported, so the error message might as well say that.  That'd be
> > a feature, not a bug.  Compare:
> >
> > % ssh $foo svn info --show-item=dept
> > svn: E205000: 'dept' is not a valid value for --show-item; did you mean
> > 'depth'?
> > % ssh $bar svn info --show-item=dept
> > svn: E205000: 'dept' is not a valid value for --show-item
> >
> > When I'm on $bar and get that error message, I know not to bother
> > trying the correct spelling because it won't work.  Same here:  If 5.9
> > supports -f nolast, 5.10 supports -f somethingelse as well, and then
> > somebody tries -f somethingelse in 5.9, a generic error message will
> > be less helpful to them than a specific one.
> >  
> >> Also, I think rather than reusing the concept of the -f option, it
> >> would be better to use another flag (maybe -l for LASTWIDGET) which is
> >> analogous to the existing -w option:  
> >
> > _Why_ would that be better?  
> 
> (sorry if the following is a bit rambly)
> 
> Just seems more consistent to me to have -l and -w, rather than -w and
> -f nolast/unrelated/flags.

I can't parse the last word.  In any case, one of the reason I used the
same option letter is that both -f flags set bits of the same bitfield
type.  Even if right now there's no bit that both -f flags set,
I suspect one might be added in the future.  (For example, a use-case for
«zle -f nolast» doesn't seem inconceivable, even if I don't have one on
me.)

> The point of the generic -f is that there
> had to be some option to initiate that mode of modifying global(ish)
> state without actually calling a widget, and adding 4 options instead
> of just one option that did 4 very related things felt weird. In a
> sense -f is like -N there, it is its own mode of operation. Eg, you
> can say zle -f yank kill and both words are arguments for -f (in a
> sense).

Yes, «zle -f» is a subcommand (like «git foo»).  All zle subcommands are
named as option flags, except for the "invoke a widget" subcommand which
is anonymous.

> In this case you're only adding a single flag and that flag is related
> to an existing option, and it is (for now) just a very long way to
> spell -l. It doesn't start a separate mode of operation, just modifies
> the current operation slightly.

That's what option flags usually do.  For example, that's what the -l
flag to ls(1) does.

> It's not like we're short on letters for options in the zle widgetname
> -opt namespace either (only 4 are used, 2 of which are already capital
> letters).

The difference will come once we support both «-f nolast» and «-f foo».
Then each such foo we add will save an option letter, and the API will
be easier to remember due to using more meaningful names.

> I think if we added more flags to your -f scheme, the
> convention would by necessity be -f nolast -f unrelated, which is
> inconsistent with the zle -f flag.

They could also be «-f nolast:unrelated», just like the $PATH envvar in
relation to the $path array.  We could even teach the toplevel -f to
support this syntax alongside the existing one for consistency, or teach
it to ignore flags called "-f" in its positional arguments, or add a «-f
foo» value that's an alias of -w, etc..

Over here the jury's still out.

Cheers,

Daniel

  reply	other threads:[~2020-06-10 13:35 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-06  8:40 Daniel Shahaf
2020-06-06 11:58 ` Mikael Magnusson
2020-06-08  6:14   ` Daniel Shahaf
2020-06-08 17:52     ` Mikael Magnusson
2020-06-10 13:34       ` Daniel Shahaf [this message]
2020-06-18 11:42         ` Daniel Shahaf
2020-06-27  2:54           ` Daniel Shahaf
2020-06-27  3:02             ` Daniel Shahaf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200610133452.025cfc54@tarpaulin.shahaf.local2 \
    --to=d.s@daniel.shahaf.name \
    --cc=mikachu@gmail.com \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).