zsh-workers
 help / color / mirror / code / Atom feed
From: Bart Schaefer <schaefer@brasslantern.com>
To: Michele Venturi <dardo82@gmail.com>
Cc: Mikael Magnusson <mikachu@gmail.com>, zsh-workers@zsh.org
Subject: Re: How to fix run-help-* functions?
Date: Mon, 20 Mar 2023 18:05:41 -0700	[thread overview]
Message-ID: <CAH+w=7Zq0sMtvmrpgN18uckwQ0HZUZcQ1XykAH4Z-oY5FzvArA@mail.gmail.com> (raw)
In-Reply-To: <CA+Ds4Nu_XL9ufnX0KBZicB04kOKCft+mK2pSkkPZs-S9RWG7VQ@mail.gmail.com>

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

On Mon, Mar 20, 2023 at 7:37 AM Michele Venturi <dardo82@gmail.com> wrote:
>
> I hope that's enough context

Typically we'd like at least OS, zsh version, whether you built it
yourself or installed packaged binaries, and a copy-paste of text from
your terminal showing the command you ran and the resulting output
(edited to relevant portions if very long).

Also "functions -T run-help" (in this case) would produce xtrace
output of the function, which might help you investigate before asking
others to do so for you.

However, given we're already this far down a thread:

> I've an alias named "pkgu" set to "sudo pacman -S -y -u" and "run-help pkgu" gives
> the same error as before, can you check it on your end?

Besides not anticipating use outside widget context, some recent
changes to run-help also did not consider that aliases result in a
recursive call, or (at least) presumed that aliases would always
mention their own name somewhere in the expansion (e.g., alias
foo="sudo foo" or the like).  Same symptom, somewhat different cause.
Both stem from an attempt to optimize out a while-loop by using
${ary[(i)...]} which in turn didn't consider what happens when the (i)
search fails.

However, we're approaching a point of diminishing returns here.
Further attempts to make sense of the foregoing risk e.g. regressing
workers/31634.  It may be better to discard the error output from the
failed shift and get on with things.  After some memory-refreshment
with the mailing list archives of past changes, the problem comes down
to something like this:

Suppose we have an alias like g='git' and on the command line is "g
diff".  We'd like the run-help widget to first unpack "g" and then
analyze the full command line as "git diff" which ultimately invokes
"run-help-git diff" to finally produce the help for the git "diff"
subcommand.

So in widget context, run-help starts by looking up "g" as an alias
and makes a recursive call on "run-help git" -- which isn't good
enough, it's lost the argument "diff".  This is where getln/print -z
come in, the recursive call pulls up the original command line
(getln), discards the "g", and appends the remainder to get "git diff"
and off we go.  However, there are all sorts of complications -- what
if the alias is g='LANG=C sudo git' or some such?  Skipping arbitrary
prefix stuff to reach the "remainder" is what that "shift" is
attempting to do.

When run-help is invoked outside widget context, there's no source
line to search for the original command name.  My first pass at this
fell back on searching the arguments passed to run-help, but that
fails on the recursive call into the expansion of the alias because
the "original command" doesn't actually appear in the expansion.  It's
possible this would also fail in widget context if one alias itself
expanded to a second alias, etc.  And that doesn't even begin to get
into possible side-effects of "alias -g".

There are several other ways this can fail -- so the most reasonable
thing might be to give up if "skip the prefix" fails and treat the
command as too complex to generate help.

The attached (follows on to previous patch) might do a little better
than simply "give up", but it also might introduce some unwanted
differences.

[-- Attachment #2: rh_shift.txt --]
[-- Type: text/plain, Size: 1104 bytes --]

diff --git a/Functions/Misc/run-help b/Functions/Misc/run-help
index 14d09bd65..462044b72 100644
--- a/Functions/Misc/run-help
+++ b/Functions/Misc/run-help
@@ -58,11 +58,11 @@ do
     case $what in
     (*( is an alias for (noglob|nocorrect))*)
 	[[ ${what[(w)7]:t} != ${what[(w)1]} ]] &&
-	  run_help_orig_cmd=${what[(w)1]} run-help ${what[(w)7]:t}
+	  run_help_orig_cmd=${what[(w)1]} run-help ${what[(w)7]:t} ${(z)${what[(w)8,-1]}}
 	;;
     (*( is an alias)*)
 	[[ ${what[(w)6]:t} != ${what[(w)1]} ]] &&
-	  run_help_orig_cmd=${what[(w)1]} run-help ${what[(w)6]:t}
+	  run_help_orig_cmd=${what[(w)1]} run-help ${what[(w)6]:t} ${(z)${what[(w)7,-1]}}
 	;;
     (*( is a * function))
 	case ${what[(w)1]} in
@@ -103,7 +103,7 @@ do
 		cmd_args=( ${(z)${cmd_args:-"$*"}} )
 
                 # Discard the command itself & everything before it.
-                shift $cmd_args[(i)${run_help_orig_cmd:-$1}] cmd_args ||
+                shift $cmd_args[(i)(${run_help_orig_cmd}|$1)] cmd_args 2>/dev/null ||
                     continue
 
                 # Discard options, parameter assignments & paths.

      reply	other threads:[~2023-03-21  1:06 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-07 14:30 Michele Venturi
2023-03-07 17:13 ` Philippe Troin
2023-03-13 11:09   ` Michele Venturi
2023-03-07 17:22 ` Bart Schaefer
2023-03-07 17:31   ` Peter Stephenson
2023-03-07 17:40     ` Bart Schaefer
2023-03-07 18:34       ` Michele Venturi
2023-03-08  0:03         ` Bart Schaefer
2023-03-13 11:14       ` Michele Venturi
2023-03-13 16:06         ` Bart Schaefer
2023-03-13 18:19           ` Mikael Magnusson
2023-03-13 20:31             ` Bart Schaefer
2023-03-13 20:41               ` Bart Schaefer
2023-03-20 14:37                 ` Michele Venturi
2023-03-21  1:05                   ` Bart Schaefer [this message]

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='CAH+w=7Zq0sMtvmrpgN18uckwQ0HZUZcQ1XykAH4Z-oY5FzvArA@mail.gmail.com' \
    --to=schaefer@brasslantern.com \
    --cc=dardo82@gmail.com \
    --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).