Re: PATCH: Improve quote handling in _ant completion

[Oliver Kiddle <opk@xxxxxxx>]

Bart Schaefer wrote:
> On Wed, Nov 29, 2023 at 11:56 AM Mikael Magnusson <mikachu@xxxxxxxxx> wrote:
> >
> > >> Also I am proposing to shield 'find_targets' with '((
> > >> $#functions[find_targets] )) ||' so that users can easily override the
> > >> function.
> > >
> > > No particular objection to this, though I wonder if it should be tied
> > > to the "targets" style and _call_program (Oliver?)

You could use _call_program but given that the default in this case
involves two separate invocations of sed and a loop, allowing the
function to be overridden is probably easier for users. I'm not overly
fond of shielding functions that way so tend to only do it for functions
that someone is likely to want to replace. But no objection from me in
this instance. Note that it is more efficient to use (( $+functions[...
and not $#functions[..

It definitely would be best renamed with _ant_ as a prefix. I would just
call it _ant_targets and include the compadd line in the function.
Perhaps whoever wrote it wanted to use it outside of completion. It
could also be rewritten to use fewer subshells.

> > It should probably also be renamed to _ant_find_targets or something
> > similar, it's very unexpected that a completer pollutes the main
> > function namespace.
>
> I've lost track of whether Oliver originally wrote this completer or
> merely committed it, but I'm going to leave this bit unchanged for him
> to weigh in.

I had to consult the git history myself to get the answer to that. And
while I did originally write it, there has been considerable change
since. The original had no separate function for targets. And it's a
long time since I used ant so it's not a completer I've ever had much
reason to look back at. I agree with everything you've said in the
feedback for this patch and haven't replied before because I didn't have
anything useful to add.

Oliver