Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00 autolearn=ham autolearn_force=no version=3.4.1 From: Frank Terbeck To: Oliver Kiddle Cc: zsh-workers@zsh.org Subject: Re: [PATCHv3] Refactor baud rate completion In-Reply-To: <27858.1462193719@thecus.kiddle.eu> (Oliver Kiddle's message of "Mon, 02 May 2016 14:55:19 +0200") References: <87k2jegi5h.fsf@ft.bewatermyfriend.org> <1462109252-12482-1-git-send-email-ft@bewatermyfriend.org> <27858.1462193719@thecus.kiddle.eu> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux) Date: Tue, 03 May 2016 23:01:32 +0200 Message-ID: <87a8k6hn4j.fsf@ft.bewatermyfriend.org> MIME-Version: 1.0 Content-Type: text/plain X-Df-Sender: NDMwNDQ0 X-Seq: zsh-workers 38395 Hi Oliver, thanks for taking your time to look over the code. I'll address these as soon as I can, though on weekdays my spare time is severely limited at the moment. Oliver Kiddle wrote: > 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. Sure that's simple enough. > 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. Huh. I wasn't aware of this one at all. But it's easy enough to fix. >> +# -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. I'll take a look at _pdf and the possibility to use _description. I kind of expected there to be a way to propagate stuff to compadd, but was too lazy to find out. :) >> +# 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 Sounds reasonable. I'll take a look at this as well. Regards, Frank