All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Florian Gamböck" <mail@floga.de>
To: "SZEDER Gábor" <szeder.dev@gmail.com>
Cc: Git mailing list <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	Stefan Beller <sbeller@google.com>
Subject: Re: [PATCH v2 1/1] completion: load completion file for external subcommand
Date: Thu, 19 Apr 2018 21:07:25 +0200	[thread overview]
Message-ID: <20180419190725.GA8555@furore> (raw)
In-Reply-To: <CAM0VKj=pDVxfJtUZx7c6uCmPxwQFPBOQYdd7NH=YnVG86iK0Pw@mail.gmail.com>

On 2018-04-18 21:51, SZEDER Gábor wrote:
> On Tue, Apr 10, 2018 at 10:28 PM, Florian Gamböck <mail@floga.de> 
> wrote:
>> Adding external subcommands to Git is as easy as to put an executable 
>> file git-foo into PATH. Packaging such subcommands for a Linux 
>> distribution can be achieved by unpacking the executable into 
>> /usr/bin of the user's system. Adding system-wide completion scripts 
>> for new subcommands, however, can be a bit tricky.
>>
>> Since bash-completion started to use dynamical loading of completion 
>> scripts since v1.90 (preview of v2.0),
>
> I believe the main bash-completion repository can be found at:
>
>  https://github.com/scop/bash-completion.git
>
> This repository still contains the branch 'dynamic-loading'; for the 
> record it points to 3b029892f6f9db3b7210a7f66d636be3e5ec5fa2.
>
> Two commits on that branch are worth mentioning:
>
>   20c05b43 (Load completions in separate files dynamically, get rid of
>             have()., 2011-10-12)
>   5baebf81 (Add _xfunc for loading and calling functions on demand,
>             use it in apt-get, cvsps, rsync, and sshfs., 2011-10-13)

Nice, thanks for the pointers!

>> (...)
>>
>> I think the easiest method is to use a function that is defined by 
>> bash-completion v2.0+, namely __load_completion.
>
> This is wrong, __load_completion() was introduced in cad3abfc 
> (__load_completion: New function, use in _completion_loader and 
> _xfunc, 2015-07-15), and the first release tag containg it is '2.2' 
> from 2016-03-03.

Dang, I thought it was introduced at the same time. Sorry for that. I 
guess, 2016 is a bit too young to take it for granted then?

> The release tags '1.90' and '2.0' are from 2011-11-03 and 2012-06-17, 
> respectively.  This leaves a couple of years long hole where 
> completions were already loaded dynamically but there was no 
> __load_completion() function.
>
> Would it be possible to use _xfunc() instead to plug that hole?  It 
> seems the be tricky, because that function not only sources but also 
> _calls_ the completion function.

But isn't this exactly what we want? Lucky us, we can replace the whole 
if-fi block with a simpler:

    _xfunc git-$command $completion_func 2>/dev/null && return

If _xfunc is not defined -- as in, bashcomp is not installed / loaded -- 
then the return will not get called and the original completion will 
continue:

    declare -f $completion_func >/dev/null 2>/dev/null &&
        $completion_func && return

Since this would be redundant, we could define a fall-back for _xfunc 
like so:

    declare -f _xfunc || _xfunc() {
        declare -f $completion_func >/dev/null 2>/dev/null &&
            $completion_func && return
    }

This way, we retain the "old" behavior and get dynamic loading if 
bashcomp is available. The actual call to get the completions would just 
be _xfunc like in my first example above.

What do you think?

-- 
Regards

Florian

  reply	other threads:[~2018-04-19 19:07 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-10 20:28 [PATCH v2 0/1] completion: dynamic completion loading Florian Gamböck
2018-04-10 20:28 ` [PATCH v2 1/1] completion: load completion file for external subcommand Florian Gamböck
2018-04-18 19:51   ` SZEDER Gábor
2018-04-19 19:07     ` Florian Gamböck [this message]
2018-04-23 15:12       ` SZEDER Gábor
2018-04-23 17:32         ` Florian Gamböck
2018-04-25 14:40         ` SZEDER Gábor
2018-04-29 11:15           ` Florian Gamböck
2018-04-29 13:08             ` SZEDER Gábor
2018-04-29 14:09               ` Florian Gamböck

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=20180419190725.GA8555@furore \
    --to=mail@floga.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sbeller@google.com \
    --cc=szeder.dev@gmail.com \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.