zsh-workers
 help / color / mirror / code / Atom feed
From: Oliver Kiddle <opk@zsh.org>
To: "Jun. T" <takimoto-j@kba.biglobe.ne.jp>
Cc: zsh-workers@zsh.org
Subject: Re: [PATCH 0/6] Update coreutils command completions
Date: Wed, 01 Nov 2023 01:12:04 +0100	[thread overview]
Message-ID: <26860-1698797524.836640@xoJh.GWYD.LK_D> (raw)
In-Reply-To: <4917B5AE-3449-49BC-94D3-F20FFEB16D3F@kba.biglobe.ne.jp>

On 9 Aug, "Jun. T" wrote:
> The second hunks in the patches for _env and _watch below are (possible)
> fix for a problem that existed before his patch.
> _env and _watch call _normal to complete a command and args, but since
> they are passed to execve() or 'sh -c', it would make no sense to complete
> zsh bultins or functions etc. I replaced
>    _normal
> by
>    _normal -p env/watch
> so that only external commands are offered. Is this a correct use of the
> option -p?

This breaks env completion. It was relying on _normal to complete
environment variables which is fairly fundamental to the use of env.

This use of _normal is fairly new. There probably is a need for a
simple way to get _normal without builtins, aliases, functions etc.
This mechanism with -p seems to have resulted from quite a number of
separate patches, each of which seemed very reasonable when taken alone
but the end effect doesn't really seem ideal. Tracking precommands
could be useful for things like noglob but isn't really for things like
env and watch. So it is being used for things that aren't conventional
precommands to get the effect of external commands only. Every use of
-p is passed $service - if that will always be the case, why require
it. I think I'd prefer a more explicit option to _normal - -e perhaps
for external commands (as for _command_names) and perhaps -E to allow
variable assignments.

In many cases, we're actually relying on later fallbacks to _default for
completing values. _default only does this for magicequalsubst so if you
unset that option, variable value completion for commands like env is
broken. _normal (or _command_names) should be doing this explicitly.

Oliver


      reply	other threads:[~2023-11-01  0:12 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-02  6:21 Shohei YOSHIDA
2023-08-02  6:21 ` [PATCH 1/6] Update head completion Shohei YOSHIDA
2023-08-02  6:21 ` [PATCH 2/6] Update date completion Shohei YOSHIDA
2023-08-02  6:21 ` [PATCH 3/6] Update env completion Shohei YOSHIDA
2023-08-02  6:21 ` [PATCH 4/6] Update tail completion Shohei YOSHIDA
2023-08-02  6:21 ` [PATCH 5/6] Update tr completion Shohei YOSHIDA
2023-08-02  6:21 ` [PATCH 6/6] Update wc completion Shohei YOSHIDA
2023-08-06 20:26 ` [PATCH 0/6] Update coreutils command completions Bart Schaefer
2023-08-09 14:09   ` Jun. T
2023-11-01  0:12     ` Oliver Kiddle [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=26860-1698797524.836640@xoJh.GWYD.LK_D \
    --to=opk@zsh.org \
    --cc=takimoto-j@kba.biglobe.ne.jp \
    --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).