git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: "SZEDER Gábor" <szeder.dev@gmail.com>,
	"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: Wed, 25 Apr 2018 16:40:56 +0200	[thread overview]
Message-ID: <CAM0VKj=MVY15SbRanfhXP3pPjNEuhZ=dbQmhXox6n5AOFyz2oQ@mail.gmail.com> (raw)
In-Reply-To: <CAM0VKjmJBzBFDAi1FQAgytb82-Q7JhO+aOfKi=YBZ3YtNxwPTQ@mail.gmail.com>

On Mon, Apr 23, 2018 at 5:12 PM, SZEDER Gábor <szeder.dev@gmail.com> wrote:
> On Thu, Apr 19, 2018 at 9:07 PM, Florian Gamböck <mail@floga.de> wrote:
>> On 2018-04-18 21:51, SZEDER Gábor wrote:
>>> 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)

>>>> 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.

>>> 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?
>
> No, that's definitely not what we want.

In my previous emails I overlooked the _completion_loader() helper
function.

It seems that this function does almost exactly what we want.  It was
introduced along with dynamic completion loading back in 20c05b43, so
it's available for us even in older LTS/Enterprise releases.  Since
cad3abfc it's a wrapper around __load_completion() and thus benefits
from all the improvements, notably searching for completion scripts in
a user-specified directory ($BASH_COMPLETION_USER_DIR) or in the
user's home directory ($XDG_DATA_HOME or ~/.local/...) as well.  It
loads the matching completion script, but does not call the completion
function unconditionally.

The "almost" refers to he case when _completion_loader() can't find a
completion script with a matching name to load, and then registers the
_minimal() completion function for the given command to do basic path
completion as fallback.  I don't think this matters in practice,
because in this case the given command is a git command in its dashed
form, e.g. 'git-diff-index', and those have been deprecated for a long
time.

I think all you need to do is run a
s/__load_completion/_completion_loader/ on your patch and update the
commit message with relevant bits from the above discussion.

  parent reply	other threads:[~2018-04-25 14:41 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
2018-04-23 15:12       ` SZEDER Gábor
2018-04-23 17:32         ` Florian Gamböck
2018-04-25 14:40         ` SZEDER Gábor [this message]
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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAM0VKj=MVY15SbRanfhXP3pPjNEuhZ=dbQmhXox6n5AOFyz2oQ@mail.gmail.com' \
    --to=szeder.dev@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sbeller@google.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).