zsh-workers
 help / color / mirror / code / Atom feed
From: Oliver Kiddle <opk@zsh.org>
To: Marlon Richert <marlon.richert@gmail.com>
Cc: Zsh hackers list <zsh-workers@zsh.org>
Subject: Re: Feature Patch: Use completion to view parameter values
Date: Fri, 02 Apr 2021 02:50:51 +0200	[thread overview]
Message-ID: <18618-1617324651.844569@tLsN.0hLE.FeTt> (raw)
In-Reply-To: <CAHLkEDt=LnW=ytsa31TQsVEQCV6cJfD8N0PWO5iuXGcLJ-tfCA@mail.gmail.com>

On 29 Mar, Marlon Richert wrote:
> When completing parameters and
> `zstyle -t ":completion:${curcontext}:parameters" extra-verbose`,
> display values of non-special parameters as descriptions.

Incidentally, we actually already do this for array elements (with the
verbose style) but not for associative array elements. Numeric
indices are rather less meaningful so that only needing the verbose
style while this uses extra-verbose is probably sensible.

Following is a review of the patch:

> -local expl pattern fakes faked tmp i pfilt
> +local MATCH MBEGIN MEND \
> +  disp dopt expl fakes faked i matches pattern pfilt sep tmp

It isn't important but I tend to split these up by type and use `local
-a' or `local -i'. Setting the right type can avoid certain issues such
as += using the declared type.

> +_tags parameters
> +( _tags && _requested parameters ) ||
> +  return

I'm not entirely sure this is needed. Tag loop nesting isn't especially
ideal and helpers like _parameters are only called from places that
already have one. But it perhaps needs a little testing. I'm aware that
the old function had this in the form of _wanted.

> -_wanted parameters expl parameter \
> -    compadd "$@" -Q - \

Note the old function was passing "$@" which you've lost. This does
matter. Passed descriptions get thrown away, similarly pressing [ to
auto-remove space suffixes on arrays. "$@" is typically passed before
"$expl[@]".

The use of double indentation for continuation lines in the removed
lines is intentional by the way, see Etc/completion-style-guide

> +_description parameters expl parameter
> +compadd "$expl[@]" -O matches - \
> +  "${(@M)${(@k)parameters[(R)${pattern[2]}~*local*]}:#${~pfilt}*}" \
> +  "$fakes[@]" \
> +  "${(@)${(@M)faked:#${~pattern[2]}}%%:*}"

The faked parameters are not going to have a value. You can just add a
separate compadd for them right at the end (take care over return
status then). It will also then need to check if any faked parameters are
duplicates of real ones otherwise they'll appear twice.

> +
> +if zstyle -t ":completion:${curcontext}:parameters" extra-verbose; then
> +  zstyle -s ":completion:${curcontext}:parameters" list-separator sep ||
> +    sep=--
> +  zformat -a disp " $sep " \
> +    ${matches[@]:/(#m)*/"${MATCH}:${${parameters[$MATCH]:#*special*}:+${(Pkv@q+)MATCH}}"}

We should probably also honour the typeset -H flag so the pattern would
need to be *(hideval|special)*

By not using _describe, this has the advantage that if the values are
all short, it can use columns. Unfortunately, many variables force it to
be a single column and then many lines have only a single short
parameter on their own. Compounding this, the parameters ! - ? @ * # $
and 0 get sorted near the top each getting a whole line.

_describe does the matches with a description first with the -l option
to compadd and then adds in undescribed matches. Perhaps this can be
presented better even if the parameters need to be spliced up.

For my own setup, I'd likely limit this style to when at least a few
characters have been typed which would avoid this; something like:
  zstyle -e ':completion:*:parameters' extra-verbose 'reply=( $(( ($#PREFIX+$#SUFFIX) > 2 )) )'

The formatting for arrays, associative arrays and empty strings could
perhaps also be nicer. Perhaps that's something for a future update.

> -0:-F doesn't break _sequence
> +0:-F doesn’t break _sequence

This is a somewhat separate change. Do we want to be using unicode here?

And, thanks. This looks useful.

Oliver


  parent reply	other threads:[~2021-04-02  0:51 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-28 20:53 Marlon Richert
2021-03-29  7:39 ` Daniel Shahaf
2021-03-29 11:55   ` Marlon Richert
2021-03-29 17:11     ` Daniel Shahaf
2021-03-29 17:20       ` Bart Schaefer
2021-03-29 18:14         ` Daniel Shahaf
2021-03-29 20:00           ` Marlon Richert
2021-03-29 20:05             ` Daniel Shahaf
2021-03-29 20:35               ` Marlon Richert
2021-04-01  4:28                 ` Marlon Richert
2021-04-01 18:40                   ` Daniel Shahaf
2021-04-02  0:50                 ` Oliver Kiddle [this message]
2021-04-10 20:20                   ` Lawrence Velázquez
2021-04-11 20:06                     ` Marlon Richert
2021-04-11 21:24                     ` Patch bumping (was Re: Feature Patch: Use completion to view parameter values) Bart Schaefer
2021-04-12  8:18                       ` Marlon
2021-04-13 12:32                         ` Daniel Shahaf
2021-04-13 18:08                           ` Lawrence Velázquez
2021-04-15  9:39                             ` [META] Tone of voice / Writing style in patch reviews (was Re: Patch bumping) Marlon
2021-04-15 10:33                               ` zeurkous
2021-04-13 13:35                         ` Patch bumping (was Re: Feature Patch: Use completion to view parameter values) Daniel Shahaf
2021-04-13 21:31                           ` Lawrence Velázquez
2021-04-13 21:50                             ` Bart Schaefer
2021-04-14 12:52                             ` Daniel Shahaf
2021-04-13  2:47                       ` Lawrence Velázquez
2021-04-12 20:22                   ` Feature Patch: Use completion to view parameter values Marlon
2021-04-12 21:49                     ` Bart Schaefer
2021-04-13  4:50                       ` Marlon Richert
2021-03-30  5:41           ` Mikael Magnusson
2021-03-31 22:55             ` Daniel Shahaf
2021-03-31 23:03               ` Daniel Shahaf
2021-03-29 20:10         ` Peter Stephenson
2021-03-29 11:48 ` Mikael Magnusson
2021-03-29 12:06   ` Marlon Richert
2021-03-29 12:07     ` Marlon Richert

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=18618-1617324651.844569@tLsN.0hLE.FeTt \
    --to=opk@zsh.org \
    --cc=marlon.richert@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).