Subject: Re: [PATCH] add-zle-hook-widget
From: Daniel Shahaf <[email-masked]>
To: [email-masked]
Date: Tue, 05 Jul 2016 04:57:57 +0000
List-Id: Zsh Workers List <zsh-workers.zsh.org>

Bart Schaefer wrote on Fri, Jul 01, 2016 at 13:11:58 -0700:
> On Jun 21,  1:41am, Daniel Shahaf wrote:
> } Subject: Re: [PATCH] add-zle-hook-widget
> }
> } Bart Schaefer wrote on Thu, Jun 16, 2016 at 22:20:55 -0700:
> } > I think this is better than the situation with add-zsh-hook where
> } > somebody has to be sure ALL the add-* calls are made in exactly the
> } > right sequence ... or else has to know the implementation and muck
> } > with it directly.
> } 
> } Agreed, although there _is_ something to be said for the "or else"
> } alternative: it is a thin/KISS solution, "here's the explicit order
> } hooks will be called in, have a field day".
> 
> You can't have it both ways -- either the implementation must be
> documented, which you objected to, or it has to be possible to do
> what's needed without knowing.

I haven't changed my opinion; I was simply trying to say that each of
these options has its merits.

> } Perhaps we should ditch indexing and use topological sorting [tsort(1)
> } is in POSIX] instead: then we can let registrants specify "before:" and
> } "after:" clauses
> 
> I don't think we want to get into the business of storing a partial
> orderting and attempting to resolve it to a total one.  For one thing,
> more information arrives every time add-zsh-hook is called.  Redo the
> whole tsort each time?

Considering that vcs_info works, and is presumably more expensive than
a tsort, we could probably do the tsort every precmd() and nobody would
complain (*cough* cygwin *cough*).

Or we could use a just-in-time approach: have add-zle-hook-widget just
stash $argv[2,-1] somewhere and invalidate the cache, and have azhw:$1()
tsort the somewhere, cache the resulting sorted order, and thereafter
use the cached answer.  (But that may be a premature optimisation...)

> } so B could do "add-zsh-hook zle-line-init B_hook before:A_hook"
> 
> How does that differ from:
> 
> } Having (B) unhook/rehook (A)'s hook sounds like a recipe for
> } hard-to-diagnose bugs.
> 

The difference is whether the order of A_hook relative to hooks *other
than* B_hook may be affected.

If B changes the index A_hook is registered at, that order may be
affected.  If B adds a "after:B_hook" constraint to A_hook, that order
shouldn't be affected [unless the additional constraint creates
a circular dependency].

> ??  It's just the internals heuristically doing the unhook/re-hook at
> B's behest, instead of B doing it explicitly.

See previous paragraphs.

> Still, there might be something.  Just thinking aloud:
> 
> Firstly, I chose the syntax "index:name" to allow multiple items to be
> added at once, but add-zsh-hook doesn't work that way so there's no
> longer any reason for add-zle-hook-widget to work that way.  So let's
> make the syntax be
> 	add-zle-hook-widget HOOK WIDGETNAME [INDEX]
> instead of embedding the index in the widgetname.
> 

+1

> (This might be tricky/confusing given that the widget list would still
> need to be stored with each index attached.)

I'm not sure what's tricky/confusing here.?

> Secondly, the index could be symbolic as you suggest; perhaps first,
> last, "before:otherwidget" and "after:otherwidget".  The difference
> from a topological sort would be that before/after apply immediately,
> so if there is no "otherwidget" present yet, they mean nothing.  You
> still have to call add-zle-hook-widget in some kind of sequence, but
> you don't have to know what index was assigned to otherwidget.  Also,
> "otherwidget" should be a pattern.
> 
> Thirdly, allow multiple symbolic indices, to cover "before:X after:Y".
> Try them until one fails, ignoring any that are meaningless.  If all
> succeed, add the hook, else report error?
> 

This sounds exactly like topological sort except that the before:/after:
constraints may only refer to already-registered widgets, and
consequently, the responsibility for declaring interdependencies between
two plugins lies entirely with the plugin sourced latter of the two.

In other words: a plugin cannot declare what order its hook should be
run in relative to plugins that have not yet been sourced.  Perhaps
virtual sequence points (see next paragraphs) can serve instead,
though.

> One other thought -- it probably doesn't work to have a single sequence
> point for e.g. whether buffer modification happens.
> 

I was thinking that would be a convention, not enforced by code.  For
example, zsh-syntax-highlighting wants to run once BUFFER is settled
down in its final form, but doesn't care which hooks are installed that
modify BUFFER.  What arguments should it pass to add-zle-hook-widget then?

This may be comparable to the 'first' / 'last' ordering constraints you
described.

> } I'm not sure whether this is simpler or more complicated than indices.
> 
> Well, it sounds a lot more complicated to me, but it depends on whether
> you mean complicated for the user to figure out beforehand what index to
> use, or complicated for the shell to manage and the user to understand
> afterward why a particular order of execution was chosen.
> 

I meant "is tsort the right way to model and solve this problem".

> As tsort(1) itself says:
> > Note that for a given partial ordering, generally there is no unique
> > total ordering.
> 
> That means the widgets might start running in different order after a
> new hook is added, for no reason the user is able to plainly see.  I like
> the notion that a known total ordering can be imposed without having to
> express increasingly detailed partial orders for each new addition.

Fair point.

Note there are alternatives to specifying detailed partial orders: for
example, one could hook a single function that invokes a few other ones:
.
    order-sensitive() {
       local w
       for w in foo bar baz; do zle -- $w -Nw -- "$@"; done
    }
    add-zle-hook-widget line-init order-sensitive
    add-zle-hook-widget line-init qux             # qux can be ordered either before or after the foo,bar,baz group

Cheers,

Daniel