From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: git@vger.kernel.org, Luke Shumaker <lukeshu@lukeshu.com>,
Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 1/2] completion: graduate out of contrib
Date: Mon, 14 Jun 2021 16:12:53 +0200 [thread overview]
Message-ID: <87wnqwzg51.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <20210614043450.1047571-2-felipe.contreras@gmail.com>
On Sun, Jun 13 2021, Felipe Contreras wrote:
> These have been stable and widely used for quite a long time, they even
> have tests outside of the contrib area, and most distributions ship
> them, so they can be considered part of the core already.
>
> We should be consistent and either we move the tests to contrib, or we
> move the completions out of contrib.
>
> Let's move them out of contrib and provide an installation target
> install-extra.
>
> By default bash-completion installs the completions to
> $(pkgdatadir)/completions, which is
> $(prefix)/share/bash-completion/completions. And since most distributions do
> not change this, it is obviously the right default that distributions
> can override with bashcompdir.
>
> By default zsh looks for completions in
> $(prefix)/share/zsh/site-functions.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> Makefile | 3 +++
> extra/Makefile | 17 +++++++++++++++++
Please let's not continue following the IMO anti-pattern of having these
sub-Makefiles. Let's just add the target to the top-level Makefile.
See e.g. the recent discussion starting at
https://lore.kernel.org/git/87pmz4ig4o.fsf@evledraar.gmail.com/ I also
have some WIP work to un-split most of this to e.g. make "install"
follow the normal quiet rules, if we're invoking those in a sub-Makefile
that becomes much more difficult....
> .../completion/git-completion.bash | 0
> .../completion/git-completion.zsh | 0
> {contrib => extra}/completion/git-prompt.sh | 0
> t/t9902-completion.sh | 8 ++++----
> t/t9903-bash-prompt.sh | 2 +-
> 7 files changed, 25 insertions(+), 5 deletions(-)
> create mode 100644 extra/Makefile
> rename {contrib => extra}/completion/git-completion.bash (100%)
> rename {contrib => extra}/completion/git-completion.zsh (100%)
> rename {contrib => extra}/completion/git-prompt.sh (100%)
>
> diff --git a/Makefile b/Makefile
> index c3565fc0f8..5bb03808b3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3105,6 +3105,9 @@ quick-install-man:
> quick-install-html:
> $(MAKE) -C Documentation quick-install-html
>
> +install-extra:
> + $(MAKE) -C extra install
> +
>
>
> ### Maintainer's dist rules
> diff --git a/extra/Makefile b/extra/Makefile
> new file mode 100644
> index 0000000000..26d8be55b0
> --- /dev/null
> +++ b/extra/Makefile
> @@ -0,0 +1,17 @@
> +bashcompdir = $(sharedir)/bash-completion/completions
> +
> +DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
> +sharedir_SQ = $(subst ','\'',$(sharedir))
> +bashcompdir_SQ = $(subst ','\'',$(bashcompdir))
> +gitexec_instdir_SQ = $(subst ','\'',$(gitexec_instdir))
> +
> +INSTALL ?= install
...and a large part of the Makefile is just things like this that we'd
mostly/entirely get for free.
> +
> +all:
> +
> +install: install-completion
> +
> +install-completion:
> + $(INSTALL) -D -m 644 completion/git-completion.bash '$(DESTDIR_SQ)$(bashcompdir_SQ)'/git
> + $(INSTALL) -D -m 644 completion/git-prompt.sh '$(DESTDIR_SQ)$(sharedir_SQ)'/git-core/git-prompt.sh
> + $(INSTALL) -D -m 644 completion/git-completion.zsh '$(DESTDIR_SQ)$(sharedir_SQ)'/zsh/site-functions/_git
> diff --git a/contrib/completion/git-completion.bash b/extra/completion/git-completion.bash
> similarity index 100%
> rename from contrib/completion/git-completion.bash
> rename to extra/completion/git-completion.bash
> diff --git a/contrib/completion/git-completion.zsh b/extra/completion/git-completion.zsh
> similarity index 100%
> rename from contrib/completion/git-completion.zsh
> rename to extra/completion/git-completion.zsh
> diff --git a/contrib/completion/git-prompt.sh b/extra/completion/git-prompt.sh
> similarity index 100%
> rename from contrib/completion/git-prompt.sh
> rename to extra/completion/git-prompt.sh
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index cb057ef161..32601b755d 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -36,7 +36,7 @@ complete ()
> GIT_TESTING_ALL_COMMAND_LIST='add checkout check-attr rebase ls-files'
> GIT_TESTING_PORCELAIN_COMMAND_LIST='add checkout rebase'
>
> -. "$GIT_BUILD_DIR/contrib/completion/git-completion.bash"
> +. "$GIT_BUILD_DIR/extra/completion/git-completion.bash"
>
> # We don't need this function to actually join words or do anything special.
> # Also, it's cleaner to avoid touching bash's internal completion variables.
> @@ -2383,14 +2383,14 @@ test_expect_success 'git clone --config= - value' '
> test_expect_success 'sourcing the completion script clears cached commands' '
> __git_compute_all_commands &&
> verbose test -n "$__git_all_commands" &&
> - . "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" &&
> + . "$GIT_BUILD_DIR/extra/completion/git-completion.bash" &&
> verbose test -z "$__git_all_commands"
> '
>
> test_expect_success 'sourcing the completion script clears cached merge strategies' '
> __git_compute_merge_strategies &&
> verbose test -n "$__git_merge_strategies" &&
> - . "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" &&
> + . "$GIT_BUILD_DIR/extra/completion/git-completion.bash" &&
> verbose test -z "$__git_merge_strategies"
> '
>
> @@ -2399,7 +2399,7 @@ test_expect_success 'sourcing the completion script clears cached --options' '
> verbose test -n "$__gitcomp_builtin_checkout" &&
> __gitcomp_builtin notes_edit &&
> verbose test -n "$__gitcomp_builtin_notes_edit" &&
> - . "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" &&
> + . "$GIT_BUILD_DIR/extra/completion/git-completion.bash" &&
> verbose test -z "$__gitcomp_builtin_checkout" &&
> verbose test -z "$__gitcomp_builtin_notes_edit"
> '
> diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
> index bbd513bab0..784e523fd4 100755
> --- a/t/t9903-bash-prompt.sh
> +++ b/t/t9903-bash-prompt.sh
> @@ -10,7 +10,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
> . ./lib-bash.sh
>
> -. "$GIT_BUILD_DIR/contrib/completion/git-prompt.sh"
> +. "$GIT_BUILD_DIR/extra/completion/git-prompt.sh"
>
> actual="$TRASH_DIRECTORY/actual"
> c_red='\\[\\e[31m\\]'
next prev parent reply other threads:[~2021-06-14 14:16 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-14 4:34 [PATCH 0/2] extra: new concept of extra components Felipe Contreras
2021-06-14 4:34 ` [PATCH 1/2] completion: graduate out of contrib Felipe Contreras
2021-06-14 14:12 ` Ævar Arnfjörð Bjarmason [this message]
2021-06-16 20:09 ` Felipe Contreras
2021-06-14 4:34 ` [PATCH 2/2] git-new-workdir: " Felipe Contreras
2021-06-14 14:18 ` [PATCH 0/2] extra: new concept of extra components Ævar Arnfjörð Bjarmason
2021-06-16 0:40 ` Junio C Hamano
2021-06-16 20:48 ` Felipe Contreras
2021-06-16 20:28 ` Felipe Contreras
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=87wnqwzg51.fsf@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=felipe.contreras@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=lukeshu@lukeshu.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 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 NNTP newsgroup(s).