Stefan Saasen <ssaa...@atlassian.com> wrote:
>Thanks for the review David, much appreciated.
>> I think this line was already too long in its current form. Would
>> splitting up this long line?
>I've updated the patch and had a look at how to avoid repeating the
>> ... follow it up with a change that generalizes the "list
>> of tools" thing so that it can be reused here, possibly. The
>> show_tool_help() function, as used by "git difftool --tool-help" and
>> "git mergetool --tool-help", might be a good place to look for
>> We were able to eliminate duplication in the docs (see the handling
>> for $(mergetools_txt) in Documentation/Makefile) so it'd be nice if
>> could do the same for git-completion.bash, somehow.
>I can think of a number of approaches and I would like to get some
>Firstly I think a similar solution to how the duplication is avoided in
>documentation can't be easily applied to the completion script. Looking
>script itself (and/or usage docs like
>http://git-scm.com/book/en/Git-Basics-Tips-and-Tricks) the recommended
>using it is by copying the script as-is. That means there won't be a
>we could rely on unless I've overlooked something?
>That leaves a different approach (run- vs. build time) where I can
>think of two
>The first would be similar to what is being done at the moment by
>the MERGE_TOOLS_DIR and in addition considering any custom merge tools
>configured. I'm working with the premise that it is a reasonable
>that users of the git completion script have a git installation
>though they may have gotten the script by other means.
>For users to still be able to install the script by simply copying it
>to any location
>on the filesystem the list generation function(s) would either have to
>from the git installation or duplicated. I suppose the former would
>take into account that the completion script doesn't necessarily
>installed version of git with some potential brittleness around
>relying on external
>files and directories. The latter doesn't buy us anything as it
>more code than the current list of available mergetools.
>The second approach would be to do something similar to resolving the
>strategies (in __git_list_merge_strategies) by parsing the output of
>merge tool --tools-help` option with a very similar disadvantage that
>on the textual output of the help command and doesn't work outside of a
>I'm currently leaning towards the last approach as it seems less
>implementation details but it doesn't look ideal either and I may be
>another approach that would be better suited.
I agree that this seems like the way to go. Perhaps we can add git
mergetool/difftool --list-tools which can print the available tools so that the
completion can use it.
>> It might be worth leaving the git-completion.bash bits alone in this
>> first patch and follow it up with a change that generalizes the "list
>> of tools" thing so that it can be reused here, possibly.
>To decouple this and adding the diffmerge merge tool option, I'd rather
>git-completion change part of the patch. That way the patch is self
>and covers the change including the completion using the current
>doesn't rely on the duplication change. Any concerns around that,
>resend the patch with only the long line fixed?
That sounds good, we can keep these as separate patches.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html