Re: [PATCH] mergetools/diffmerge: support DiffMerge as a git mergetool

>Thanks for the review David, much appreciated.
>
>> I think this line was already too long in its current form.  Would
>you mind
>> splitting up this long line?
>
>I've updated the patch and had a look at how to avoid repeating the
>list of
>available merge/difftools.
>
>> ... 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
>> inspiration.
>
>> We were able to eliminate duplication in the docs (see the handling
>> for $(mergetools_txt) in Documentation/Makefile) so it'd be nice if
>we
>> could do the same for git-completion.bash, somehow.
>
>I can think of a number of approaches and I would like to get some
>feedback.
>
>Firstly I think a similar solution to how the duplication is avoided in
>the
>documentation can't be easily applied to the completion script. Looking
>at the
>script itself (and/or usage docs like
>http://git-scm.com/book/en/Git-Basics-Tips-and-Tricks) the recommended
>way of
>using it is by copying the script as-is. That means there won't be a
>build step
>we could rely on unless I've overlooked something?
>
>That leaves a different approach (run- vs. build time) where I can
>think of two
>possible solutions.
>The first would be similar to what is being done at the moment by
>looking at
>the MERGE_TOOLS_DIR and in addition considering any custom merge tools
>configured. I'm working with the premise that it is a reasonable
>assumption
>that users of the git completion script have a git installation
>available even
>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
>be sourced
>from the git installation or duplicated. I suppose the former would
>need to
>take into account that the completion script doesn't necessarily
>matches the
>installed version of git with some potential brittleness around
>relying on external
>files and directories. The latter doesn't buy us anything as it
>duplicates even
>more code than the current list of available mergetools.
>
>The second approach would be to do something similar to resolving the
>merge
>strategies (in __git_list_merge_strategies) by parsing the output of
>the `git
>merge tool --tools-help` option with a very similar disadvantage that
>it relies
>on the textual output of the help command and doesn't work outside of a
>git
>repository.
>
>
>I'm currently leaning towards the last approach as it seems less
>reliant on
>implementation details but it doesn't look ideal either and I may be
>missing
>another approach that would be better suited.

>
>> 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
>keep the
>git-completion change part of the patch. That way the patch is self
>contained
>and covers the change including the completion using the current
>approach and
>doesn't rely on the duplication change. Any concerns around that,
>otherwise I'll
>resend the patch with only the long line fixed?