Re: [PATCH v2 0/4] Auto-generate mergetool lists
John Keeping wrote:
On Mon, Jan 28, 2013 at 01:50:19PM -0800, Junio C Hamano wrote:
What are the situations where a valid user-defined tools is
unavailable, by the way?
The same as a built-in tool: the command isn't available.
Currently I'm extracting the command word using:
cmd=$(eval -- set -- $(git config mergetool.$tool.cmd); echo
\$1\)
Shouldnt this work?
cmd=$((git config mergetool.$tool.cmd || git config difftool.$tool.cmd)
| awk '{print $1}')
(it's slightly more complicated due to handling difftool.$tool.cmd as
well, but that's essentially it). Then it just uses the same type
$cmd test as for built-in tools.
I don't know if there's a better way to extract the first word, but
that's the best I've come up with so far.
John
--
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
Re: [PATCH v2 0/4] Auto-generate mergetool lists
On Tue, Jan 29, 2013 at 12:56:58PM +0100, Joachim Schmitz wrote:
John Keeping wrote:
Currently I'm extracting the command word using:
cmd=$(eval -- set -- $(git config mergetool.$tool.cmd); echo
\$1\)
Shouldnt this work?
cmd=$((git config mergetool.$tool.cmd || git config difftool.$tool.cmd)
| awk '{print $1}')
That doesn't handle paths with spaces in, whereas the eval in a subshell
does:
$ cmd='my command $BASE $LOCAL $REMOTE'
$ echo $cmd | awk '{print $1}'
my
$ ( eval -- set -- $cmd; echo \\$1\ )
my command
John
--
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
Re: [PATCH v2 0/4] Auto-generate mergetool lists
On Tue, Jan 29, 2013 at 08:15:15AM -0800, Junio C Hamano wrote:
With any backend that is non-trivial, it would not be unusual for
the *tool.cmd to look like:
[mergetool]
mytool = sh -c '
... some massaging to prepare the command line
... to run the real tool backend comes here, and
... then ...
my_real_tool $arg1 $arg2 ...
'
and you will end up detecting the presence of the shell, which is
not very useful.
I think it is perfectly fine to say you configured it, so it must
exist; it may fail when we try to run it but it is your problem.
It is simpler to explain and requires one less eval.
I think you're right. The even worse case from this point of view is if
you configure it as:
[mergetool]
mytool = 'f() {
... code to actually run the tool here ...
}; f $BASE $REMOTE $LOCAL $MERGED'
which results in a false unavailable rather than just a useless check.
John
--
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
Re: [PATCH v2 0/4] Auto-generate mergetool lists
From: David Aguilar dav...@gmail.com Sent: Monday, January 28, 2013 12:52 AM This is round two of this series. I think this touched on everything brought up in the code review. 4/4 could use a review as I'm not completely familiar with the makefile dependencies, though it seems to work correctly. Does this 4/4 have any effect on the Msysgit / Git for Windows documentation which simply refers [IIRC] to HTML documenation made by Junio? That is, how easy is it to create a 'default' set of docs, rather than personalised documenation. Or have I misunderstood how it is working? David Aguilar (4): mergetool--lib: Simplify command expressions mergetool--lib: Improve the help text in guess_merge_tool() mergetool--lib: Add functions for finding available tools doc: Generate a list of valid merge tools Documentation/.gitignore | 1 + Documentation/Makefile | 22 +++- Documentation/diff-config.txt | 13 ++--- Documentation/merge-config.txt | 12 ++--- git-mergetool--lib.sh | 116 ++--- 5 files changed, 96 insertions(+), 68 deletions(-) -- 1.8.0.13.g3ff16bb -- 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 - No virus found in this message. Checked by AVG - www.avg.com Version: 2013.0.2890 / Virus Database: 2639/6061 - Release Date: 01/27/13 -- 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
Re: [PATCH v2 0/4] Auto-generate mergetool lists
On Mon, Jan 28, 2013 at 12:20 AM, Philip Oakley philipoak...@iee.org wrote: From: David Aguilar dav...@gmail.com Sent: Monday, January 28, 2013 12:52 AM This is round two of this series. I think this touched on everything brought up in the code review. 4/4 could use a review as I'm not completely familiar with the makefile dependencies, though it seems to work correctly. Does this 4/4 have any effect on the Msysgit / Git for Windows documentation which simply refers [IIRC] to HTML documenation made by Junio? That is, how easy is it to create a 'default' set of docs, rather than personalised documenation. Or have I misunderstood how it is working? It doesn't have any effect on Msysgit. The resulting documentation lists all available tools, on all platforms. -- David -- 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
Re: [PATCH v2 0/4] Auto-generate mergetool lists
On Sun, Jan 27, 2013 at 06:41:04PM -0800, David Aguilar wrote: John, I didn't completely address your question about keeping the sort and prefix in show_tool_help() but I can stop poking at it now in case you want to start looking at what it would take to get custom tools listed in the --tool-help output. I've had a quick look and it's quite straightforward to build on top of this to get an output format like this: 'git mergetool --tool-tool' may be set to one of the following: araxis gvimdiff gvimdiff2 vimdiff vimdiff2 user-defined: mytool The following tools are valid, but not currently available: bc3 codecompare deltawalker diffuse ecmerge emerge kdiff3 meld opendiff p4merge tkdiff tortoisemerge xxdiff user-defined: mybrokentool Some of the tools listed above only work in a windowed environment. If run in a terminal-only session, they will fail. I don't think the suffix form would be too hard either - it just requires moving an explicit sort into the top-level shot_tool_help function. I'm going to hold off doing any more on this until da/mergetool-docs has graduated to next since I think it will be easier to just build on that rather than trying to put all the necessary pieces into place now. John -- 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
Re: [PATCH v2 0/4] Auto-generate mergetool lists
From: David Aguilar dav...@gmail.com Sent: Monday, January 28, 2013 9:16 AM On Mon, Jan 28, 2013 at 12:20 AM, Philip Oakley philipoak...@iee.org wrote: From: David Aguilar dav...@gmail.com Sent: Monday, January 28, 2013 12:52 AM This is round two of this series. I think this touched on everything brought up in the code review. 4/4 could use a review as I'm not completely familiar with the makefile dependencies, though it seems to work correctly. Does this 4/4 have any effect on the Msysgit / Git for Windows documentation which simply refers [IIRC] to HTML documenation made by Junio? That is, how easy is it to create a 'default' set of docs, rather than personalised documenation. Or have I misunderstood how it is working? It doesn't have any effect on Msysgit. The resulting documentation lists all available tools, on all platforms. That's useful to know. I must have misunderstood one of the earlier messages suggesting it would also list all the users other (non typical) installed mergetools and hence add them into the documentation. Philip -- 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
Re: [PATCH v2 0/4] Auto-generate mergetool lists
John Keeping j...@keeping.me.uk writes: I've had a quick look and it's quite straightforward to build on top of this to get an output format like this: 'git mergetool --tool-tool' may be set to one of the following: araxis ... vimdiff2 user-defined: mytool The following tools are valid, but not currently available: bc3 ... xxdiff user-defined: mybrokentool Some of the tools listed above only work in a windowed environment. If run in a terminal-only session, they will fail. I don't think the suffix form would be too hard either - it just requires moving an explicit sort into the top-level shot_tool_help function. I tend to think that one-tool-per-line format like the above looks nicer. What are the situations where a valid user-defined tools is unavailable, by the way? -- 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
Re: [PATCH v2 0/4] Auto-generate mergetool lists
On Mon, Jan 28, 2013 at 01:50:19PM -0800, Junio C Hamano wrote: What are the situations where a valid user-defined tools is unavailable, by the way? The same as a built-in tool: the command isn't available. Currently I'm extracting the command word using: cmd=$(eval -- set -- $(git config mergetool.$tool.cmd); echo \$1\) (it's slightly more complicated due to handling difftool.$tool.cmd as well, but that's essentially it). Then it just uses the same type $cmd test as for built-in tools. I don't know if there's a better way to extract the first word, but that's the best I've come up with so far. John -- 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
[PATCH v2 0/4] Auto-generate mergetool lists
This is round two of this series. I think this touched on everything brought up in the code review. 4/4 could use a review as I'm not completely familiar with the makefile dependencies, though it seems to work correctly. David Aguilar (4): mergetool--lib: Simplify command expressions mergetool--lib: Improve the help text in guess_merge_tool() mergetool--lib: Add functions for finding available tools doc: Generate a list of valid merge tools Documentation/.gitignore | 1 + Documentation/Makefile | 22 +++- Documentation/diff-config.txt | 13 ++--- Documentation/merge-config.txt | 12 ++--- git-mergetool--lib.sh | 116 ++--- 5 files changed, 96 insertions(+), 68 deletions(-) -- 1.8.0.13.g3ff16bb -- 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
Re: [PATCH v2 0/4] Auto-generate mergetool lists
I think our works crossed, while I was tweaking the previous series to push out as part of 'pu' you were already rerolling. Could you compare this series with what I pushed out and see if anything you missed? I think I fixed the (a b || c d) issue in the version I pushed out, but it is still there in this series. -- 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
Re: [PATCH v2 0/4] Auto-generate mergetool lists
On Sun, Jan 27, 2013 at 6:08 PM, Junio C Hamano gits...@pobox.com wrote: I think our works crossed, while I was tweaking the previous series to push out as part of 'pu' you were already rerolling. Could you compare this series with what I pushed out and see if anything you missed? I think I fixed the (a b || c d) issue in the version I pushed out, but it is still there in this series. Ah, I see. I can add the addition of preamble for use by show_tool_help() as a follow up along with using a here-doc when printing. The other diff is the Makefile dependencies. I currently have this diff against da/mergetool-docs: diff --git a/Documentation/Makefile b/Documentation/Makefile index f595d26..834ec25 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -202,7 +202,11 @@ install-html: html # # Determine include:: file references in asciidoc files. # -doc.dep : $(wildcard *.txt) build-docdep.perl +docdep_prereqs = \ + mergetools-list.made $(mergetools_txt) \ + cmd-list.made $(cmds_txt) + +doc.dep : $(docdep_prereqs) $(wildcard *.txt) build-docdep.perl $(QUIET_GEN)$(RM) $@+ $@ \ $(PERL_PATH) ./build-docdep.perl $@+ $(QUIET_STDERR) \ mv $@+ $@ I'll send three follow-up patches. - here-doc, preamble stuff, and Makefile deps, based on what's currently in pu. -- David -- 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
Re: [PATCH v2 0/4] Auto-generate mergetool lists
On Sun, Jan 27, 2013 at 6:27 PM, Junio C Hamano gits...@pobox.com wrote: David Aguilar dav...@gmail.com writes: On Sun, Jan 27, 2013 at 6:08 PM, Junio C Hamano gits...@pobox.com wrote: I think our works crossed, while I was tweaking the previous series to push out as part of 'pu' you were already rerolling. Could you compare this series with what I pushed out and see if anything you missed? I think I fixed the (a b || c d) issue in the version I pushed out, but it is still there in this series. Ah, I see. I can add the addition of preamble for use by show_tool_help() as a follow up along with using a here-doc when printing. I think the progression of the series is just fine as-is with the new series you posted (I didn't amend the old one with all the suggestions I made in the review, just only with the more important ones that would affect correctness, so please consider that the changes you have in this new round that I didn't have in 'pu' are good ones to keep. Okay, cool, so no need to reroll, ya? The one thing missing in my latest series was the fixed mode_ok() which you corrected in cfb611b34089a0b5794f4ec453289a4764d94050. Let me know if there's anything else I should send out or splice together. John, I didn't completely address your question about keeping the sort and prefix in show_tool_help() but I can stop poking at it now in case you want to start looking at what it would take to get custom tools listed in the --tool-help output. -- David -- 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
Re: [PATCH v2 0/4] Auto-generate mergetool lists
David Aguilar dav...@gmail.com writes: Okay, cool, so no need to reroll, ya? It was more like please don't switch to incremental yet; I tweaked the mode_ok in your v2 and pushed out the result on 'pu' again. There may later be comments from others that make us realize some patches need to be rerolled, but nothing from me for now. Thanks. -- 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
