Re: [PATCH 4/4] _normal: Add -P to reset precommands

[Matthew Martin <phy1729@xxxxxxxxx>]

On Tue, Apr 02, 2019 at 04:27:41PM -0400, Daniel Shahaf wrote:
> dana wrote on Tue, 02 Apr 2019 17:47 +00:00:
> > On 1 Apr 2019, at 22:13, Matthew Martin <phy1729@xxxxxxxxx> wrote:
> > >+xitem(tt(_normal) [ tt(-P) | tt(-p) var(precommand) ])(
> > 
> > I know it wasn't your fault, but i think this should be item()(), not
> > xitem()(). I don't fully understand the distinction, but xitem() seems to be
> > used only when there are multiple headings (describing different ways to use
> > the command); left here, it breaks the man-page formatting for the paragraphs
> > you added.
> 
> There should be zero or more xitem() [with one pair of parens each]
> followed by exactly one item()() [with two pairs of parens, the second
> one being multiline).

Thanks for the explanation.

> Since I'm replying: I don't understand why `tt(foo) tt(bar) tt(baz)` is
> spelled with three macros; I think`tt(foo bar baz)` would be fine… but
> of course this is minor.

Because _multi_parts just above does something similar (although it's
alone in this file using that style). Fixed.

> > > completes after pre-command specifiers such as tt(nohup), removes the
> > 
> > Also not your fault, but this is the only place in the documentation
> > (including your changes) where 'pre-command' is hyphenated. Maybe fix that?

Fixed.

> > >+Append var(precommand) to the list of precommands. Should be used in
> > >+Reset the list of precommands. Should be used if completing a command
> > 
> > This clipped style (ommitting the subject of the sentence) isn't used anywhere
> > else in the documentation that i can see.

It's used with the other helper functions that describe their options in
a list (cf. _completers, _dir_sep, _email_addresses, although only -f
for _path_files). I think the imperative generally results in more
concise documentation for flags ("specifies that" doesn't add anything);
but if the declarative is preferred, I can change the patch to match.

> > >+  '*::arguments: _normal -p $service'
> > 
> > The 'arguments' here is superfluous.

Fixed.

> > idk if you planned to do another pass, but i noticed several straight-forward
> > examples of other functions that could make use of `_normal -p`: _env,

I'm planning to review uses of _normal next. This round was just
cleaning up completers that already used precommands.