Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm
Precedence: bulk
In-reply-to: <1462109252-12482-1-git-send-email-ft@bewatermyfriend.org>
From: Oliver Kiddle <okiddle@yahoo.co.uk>
References: <87k2jegi5h.fsf@ft.bewatermyfriend.org> <1462109252-12482-1-git-send-email-ft@bewatermyfriend.org>
To: zsh-workers@zsh.org
Subject: Re: [PATCHv3] Refactor baud rate completion
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <27857.1462193719.1@thecus.kiddle.eu>
Date: Mon, 02 May 2016 14:55:19 +0200
Message-ID: <27858.1462193719@thecus.kiddle.eu>

Frank Terbeck wrote:
> This adds a new helper function _baudrate and uses it in place of
> private solutions in various existing completions.

Some comments on this: the normal convention is for helper functions
to have plural names. The logic being that all baud rates are added
as matches. So this should be named _baudrates or _baud_rates.

Secondly, the convention for completion functions is two spaces for
indentation. See Etc/completion-style-guide. Annoyingly, there's quite a
few functions that don't follow this but it'd be nice if we could avoid
adding more.

> +#     -t TAG       Use TAG as the tag value in _wanted call.
> +#
> +#     -d DESC      Use DESC as the description value in _wanted call.

It is usually more flexible to just accept normal compadd options for
descriptions and tags and pass them on to compadd fairly directly. It
then will work in conjunction with other helpers like _alternative.
_pdf is a good example. Using _wanted here isn't entirely necessary:
_description would be sufficient and avoids nesting tag loops.

> +# It is also possible to override the arguments to -f, -u and -l via styles in
> +# a similar fashion:
> +#
> +#   zstyle ':completion:*:*:screen:*' max-baud-rate 9600
> +#   zstyle ':completion:*:*:screen:*' min-baud-rate 1200
> +#   zstyle ':completion:*:*:screen:*' baud-filter some_function_name

The original concept with styles was that style's could have fairly
generic names because the context allows you to select the detailed
context. So perhaps consider allowing this to work as, for example:
  zstyle ':completion:*:*:screen:*:baud-rates' max-value 9600

Oliver