Re: [PATCH] Add execute-command() widget function (was Re: [RFC][PATCH] Add change-directory() widget function)

[Daniel Shahaf <d.s@daniel.shahaf.name>]

Marlon Richert wrote on Thu, Apr 22, 2021 at 13:55:35 +0300:
> On 22 Apr 2021, at 00:58, Bart Schaefer <schaefer@brasslantern.com> wrote:
> > On Wed, Apr 21, 2021 at 2:28 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> >> Again, please wrap long lines in your prose.  This is the third time you
> >> are being asked to do so.
> 
> I would love to, but I’m not sure when it happens (everything looks fine on my end) and I haven’t found an explicit setting for it in my email client. Please let me know if this email has long lines or not. That would help me figure out which combination of settings might be causing this.

Yes, it does.

> 
> >> As to PUSHD_MINUS and PUSHD_SILENT, it would be better to give an
> >> example doesn't change them from their default values.
> 
> Why exactly?

The very next sentence (which Bart had snipped, but you should have read
anyway) points to another post of mine which gives a reason.

Moreover, that sentence was the second time in two days that I pointed
you to that specific post.  Overall, I think there have been three or
four unique pieces of feedback which you were given multiple times *this
week alone*.  We shouldn't have to repeat ourselves to you.

> On 22 Apr 2021, at 00:27, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> >> +ifnzman(noderef(Miscellaneous))). More precisely, it 
> > 
> > s/it/it:|it+DASH()-/ ?
> 
> Sorry, I’m new to Yodl. What exactly would that do and why would we need that here?
> 

It was a request to change «it» either to «it:» or to «it+DASH()-».

The former would append a colon to the paragraph.

The latter would append an em dash.  The manual sources denote an em
dash as «DASH()-» (which is poor encapsulation and a leaky abstraction,
yes).

> >> +You can use this, for example, to create key bindings that let you instantly 
> >> +change directories, even while in the middle of typing another command: 
> >> +
> >> +example(autoload -Uz execute-command
> >> +setopt autopushd pushdminus pushdsilent
> > 
> > AUTO_PUSHD is set but not used.
> 
> If I wouldn’t set it, then the names cd-backward and cd-forward  wouldn’t make any sense anymore.

Sorry, I was thinking of AUTO_CD.

The point below applies, though: it would be better to give an example
that works under the default options; in this case, to use «pushd»
rather than «cd» with an option.

> 
> > As to PUSHD_MINUS and PUSHD_SILENT, it would be better to give an
> > example doesn't change them from their default values.  That's again
> > 45149, which I mentioned just yesterday.
> 
> Let’s suppose the following:
> I rewrite the example to use `pushd +1` and `pushd -0`, so it works with default shell options.
> A novice user has PUSHD_MINUS in their config (for whatever reason; they might not even know what it does).
> They copy-paste the example.
> Now the widgets won’t work at all as expected. It’s not even the reverse.
> 

A user who doesn't know what their config does isn't the primary
use-case we should design for.  Besides, we should never recommend to
set an option globally if setting it locally would suffice — at which
point we might as well set the _default_ locally, to handle users who
(deliberately) set the option to a value other than the default.

> Setting PUSHD_MINUS in the example guarantees that it will work out of the box for novice users. (More advanced users, I’m sure, will be able to figure out how to modify it to suit their needs.)
> 

Yes, until they ssh to some other box where they have an empty .zshrc
file and don't understand why they get the non-PUSHD_MINUS behaviour.
(And for that not to happen, then they'll have to read up on
PUSHD_MINUS, which increases the learning curve — which is the point
I made in 45149.  Did you read that post?  I didn't look up those X-Seq
values for my own amusement.)

> As for PUSHD_SILENT, as I pointed out above, using `pushd -q` instead has unwanted side effects. It does not have an interchangeable alternative.

Isn't «pushd foo > /dev/null» an alternative?  And it has the obvious
advantage of not affecting other uses of pushd.

> 
> >> +zle -N cd-upward  ; cd-upward()   { execute-command cd .. }
> >> +zle -N cd-backward; cd-backward() { execute-command pushd -1 }
> >> +zle -N cd-forward ; cd-forward()  { execute-command pushd +0 }
> > 
> > s/()/+LPAR()+RPAR()/ so they don't look like yodl macros (and
> > potentially throw build-time warnings).  (For Vim users, the custom
> > ftplugin sets up concealing which does the reverse replacement while
> > editing.)
> 
> Again, sorry, I’m new to Yodl. What exactly are you suggesting?

That you literally change «()» to «+LPAR()+RPAR()» so yodl doesn't look
for 0-ary macros called «upward()», «backward()», etc., and warn on
stderr that it can't find them.  There's plenty of examples of that.

> >> +bindkey '^[^[[A' cd-upward; bindkey '^[^[OA' cd-upward
> > 
> > -1 on using random escape sequences without any explanation of what they
> > do and how to find them out.
> > 
> > Can the symbolic names of these escape sequences be used here?  At least
> > .
> >    % for k v in "${(@kv)terminfo}" ; [[ $v == (*\[A*|*OA*) ]] && echo $k 
> >    cuu1
> >    kcuu1
> > .
> > if we don't have anything more human-readable than those.  (I think this
> > was discussed somewhere in the "default zshrc" thread so I won't
> > elaborate here.)
> 
> For cuu1 and cuf1, that would work, but ^[[B and ^[[D do not have
> entries in terminfo.

They aren't used in the manual either, so why does it matter whether
they're in $terminfo or not?

> I don’t think we should use $terminfo[cuu1] and $terminfo[cuf1] in
> some examples but then hard-code ^[[B and ^[[D elsewhere. Hard-coding
> all of ^[[{A..D} would be more clear.

Disagree.  I would have proposed an alternative, but it's an academic
issue.

> I’m fine using $terminfo for kcuu1, kcud1, kcuf1 and kcub1. Using
> $key[Up] would be even better, but we cannot rely on that being set.