Re: [PATCH] Speedup bash completion loading

From: Shawn O. Pearce <spearce@spearce.org>
Date: Oct 8, 2009, 15:02 UTC
Message-ID: <20091008150206.GD9261@spearce.org>
In-Reply-To: <20091008132718.GA12161@tugrik.mns.mnsspb.ru>

Kirill Smelkov <kirr@mns.spb.ru> wrote:

Show 12 quoted lines

> diff --git a/contrib/completion/Makefile b/contrib/completion/Makefile
> new file mode 100644
> index 0000000..a0fbb66
> --- /dev/null
> +++ b/contrib/completion/Makefile
> @@ -0,0 +1,11 @@
> +all	: git-completion.bash
> +
> +
> +git-completion.bash: git-completion.bash.in git-completion.bash.generate
> +	# Generate completions for binaries we have just built
> +	PATH="$(shell pwd)/..:$$PATH" ./git-completion.bash.generate

Is only one .. enough? Isn't that putting us into the contrib directory, and therefore not finding the 'git' we just compiled?

I'm also concerned that git-completion.bash.generate requires bash to compile the completion for bash. IMHO, if we are building this code at compile time we shouldn't assume bash is available. What if this is a sandboxed build environment using another shell and /bin/bash isn't installed?

I think the git-completion.bash.generate code needs to be a bit more sh agnostic than the completion routines themselves are.

> +# pregenerated stuff (to save load time)
> +__git_merge_strategylist=__GIT_MERGE_STRATEGYLIST
> +__git_all_commandlist=__GIT_ALL_COMMANDLIST
> +__git_porcelain_commandlist=__GIT_PORCELAIN_COMMANDLIST

This also makes testing the completion a bit more difficult, now we have to build it before we can load it, making the testing cycle actually be:

  make && . git-completion.bash

We probably should place a quick comment here to remind folks that they need to build the script in order to test it properly.

-- 
Shawn.