zsh-workers
 help / color / mirror / code / Atom feed
From: Oliver Kiddle <okiddle@yahoo.co.uk>
To: gi1242+zsh@gmail.com, zsh-workers@zsh.org
Subject: Re: [patch] Completion for _deborphan and _xrandr
Date: Sun, 15 Oct 2017 23:00:24 +0200	[thread overview]
Message-ID: <3912.1508101224@thecus.kiddle.eu> (raw)
In-Reply-To: <20171015185931.ano4dq3c5tevu5jf@andrew.cmu.edu>

gi1242+zsh@gmail.com wrote:
>
> I wrote a rudimentary completion script for deborphan (which probably
> belongs with the Debian completion commands).

Thanks for this.

> I also fixed the xrandr completion to separate outputs into two groups
> (connected and disconnected). Patch, and whole file attached. If there

Is it useful to complete a disconnected output then? If it's only
relevant with some options then we could have separate states for
completing all vs. only connected outputs.

> is a preferred way to submit this (e.g. by forking a git repository) let
> me know and I would be happy to do so.

The preferred way to submit this is by posting a patch, exactly as you
have done.

I've got a couple of minor comments and suggestions on the function:

> +    _alternative \
> +	'connected:connected outputs:('${(j: :)${(uo)${(M)xrandr_output:#* connected*}%% *}}')' \
> +	'disconnected:disconnected outputs:('${(j: :)${(uo)${(M)xrandr_output:#* disconnected*}%% *}}')' \
> +	&& return 0

The normal convention is for the group descriptions to be singular: they
describe what comes on the command line rather than the matches
collectively. So, "connected output" rather than "connected outputs".

> local keep=/var/lib/deborphan/keep
> _arguments : \
>   {--help,-h}'[help]' \
>   {--status-file,-f}'[statusfile]:file:_files' \

The description here is rather terse.

>   {--exclude,-e}'[work as if packages in LIST were not installed]:LIST:' \

I'm guessing that LIST here has come from the --help output. The
description's inclusion of "LIST" makes less sense without corresponding
text saying something like [ --exclude LIST ]. The word "specify" is
often used when rewording these. Also, is it not perhaps valid to
complete a list of installed packages here. So:

  {--exclude,-e}'[work as if specified packages were not installed]:package:_sequence _deb_packages - installed' \

That is assuming a comma-separated list. If deborphan allows you to pass
  -e pkg1 -e pkg2
Then you need to prefix the _arguments spec with a *:
  \*{--exclude,-e}'[....

>   {--force-hold,-H}'[Ignore hold flags.]' \

This description is inconsistent with the earlier ones. Our normal
convention is to neither capitalise the first word nor finish with a
full stop.

>   {--priority,-p}'[PRIOR  Select only packages with priority >= PRIOR.]:PRIOR:' \

The same comment I made about LIST applies here and in some later
places. Is there a default priority. It can be good to provide hints,
perhaps:
   :priority (1-100) [50]'
I'm assuming the priorities are just numbers.

>   {--del-keep,-R}"[PKGS.. Remove PKGS from the 'keep' file.]:*:package:_values package $(< $keep)" \

Is _values really needed here or would compadd do the job?

I'd also be inclined to try to avoid it dumping error messages to the
terminal if the $keep file doesn't exist. Perhaps:
  keep=( /var/lib/deborphan/keep(N) /dev/null)
  ..
  compadd $(<$keep[1]})

Thanks again.

Oliver


  reply	other threads:[~2017-10-15 21:00 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-15 18:59 gi1242+zsh
2017-10-15 21:00 ` Oliver Kiddle [this message]
2017-10-16  0:53   ` gi1242+zsh

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=3912.1508101224@thecus.kiddle.eu \
    --to=okiddle@yahoo.co.uk \
    --cc=gi1242+zsh@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).