Re: Re*: mergetool: support --tool-help option like difftool does
Am 24.08.2012 10:31, schrieb David Aguilar: > On Thu, Aug 23, 2012 at 10:39 AM, Junio C Hamano wrote: >> David Aguilar writes: >>> Would the ability to resolve the various merge situations using >>> the command-line be a wanted addition? >>> >>> This would let a submodule or deleted/modified encountering >>> user do something like: >>> >>> $ git mergetool --theirs -- submodule >>> >>> ...and not have to remember the various git commands that it runs. >> >> Does it have to run various git commands? For a normal path, all it >> needs to do is "git checkout --theirs $path", no? >> >> What does it mean to resolve a conflicted merge of a gitlink to take >> "theirs"? We obviously would want to point the resolved gitlink at >> the submodule commit their side wants in the resulting index but what, >> if any, should we do to the submodule itself? >> >> Stepping back a bit, if there is no conflict, and as a result of a >> clean merge (this applies to the case where you check out another >> branch that has different commit at the submodule path), if gitlink >> changed to point at a different commit in the submodule, what should >> happen? >> >> If you start from a clean working tree, with your gitlink pointing >> at the commit that matches HEAD in the submodule, and if the working >> tree of the submodule does not have any local modification, it may >> be ideal to check out the new commit in the submodule (are there >> cases where "git checkout other_branch" in the superproject does not >> want to touch the submodule working tree?). >> >> There are cases where it is not possible; checking out the new >> commit in the submodule working tree may not succeed due to local >> modifications. But is that kind of complication limited to the >> merge resolution case? Isn't it shared with normal "switching >> branches" case as well? >> >> If so, it could be that your "git mergetool --theirs -- submodule" >> is working around a wrong problem, and the right solution may be to >> make "git checkout --theirs -- $path" to always do an appropriate >> thing regardless of what kind of object $path is, no? > > True. I agree. > Admittedly, I'm not a heavy submodule user myself so I > tried crafting the directory vs submodule conflict to see > what the usability is like. > > checkout --theirs and --ours could learn a few tricks. Me thinks that after I successfully taught checkout to properly recurse into submodule work trees too it should know all those tricks. In my current version it can handle directory/submodule conversions in both directions, this should be sufficient to make "checkout --theirs/--ours" work properly. Note to self: add tests for that. > When trying to choose the directory in that situation > I had to had to "git rm --cached" the submodule path > so that git would recognize that there was no longer a conflict. > > That makes sense to me because I was following along by > reading the mergetool code, but I don't think most users > would know to "git rm" a path which they intend to keep. True. But a submodule recursing checkout would do the right thing here too. > Afterwards, the .git file is left behind, which could cause > problems elsewhere since we really don't want a .git file > in that situation. Hmm, either you remove all the files tracked in the submodule together with the gitfile or you'll possibly have former submodule files lying around there too. Recursive checkout will do all that for you. > I'm not even sure what to do about the > .gitmodules file either. Maybe we should issue a warning when the .gitmodules file is not consistent with the [non]existence of a submodule in the work tree? > That said, this really isn't an issue, per say. > I first poked at it because I noticed that mergetool > still needed stdin for some things. > > It's certainly an edge case, and perhaps this just shows > that mergetool really is the right porcelain for the job > when a user runs into these types of conflicts > (the stdin thing really isn't an issue). It looks to me as if the submodule/directory conflict can be handled by a recursive checkout without having to add something to mergetool. -- 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: Re*: mergetool: support --tool-help option like difftool does
On Thu, Aug 23, 2012 at 10:39 AM, Junio C Hamano wrote: > David Aguilar writes: >> Would the ability to resolve the various merge situations using >> the command-line be a wanted addition? >> >> This would let a submodule or deleted/modified encountering >> user do something like: >> >> $ git mergetool --theirs -- submodule >> >> ...and not have to remember the various git commands that it runs. > > Does it have to run various git commands? For a normal path, all it > needs to do is "git checkout --theirs $path", no? > > What does it mean to resolve a conflicted merge of a gitlink to take > "theirs"? We obviously would want to point the resolved gitlink at > the submodule commit their side wants in the resulting index but what, > if any, should we do to the submodule itself? > > Stepping back a bit, if there is no conflict, and as a result of a > clean merge (this applies to the case where you check out another > branch that has different commit at the submodule path), if gitlink > changed to point at a different commit in the submodule, what should > happen? > > If you start from a clean working tree, with your gitlink pointing > at the commit that matches HEAD in the submodule, and if the working > tree of the submodule does not have any local modification, it may > be ideal to check out the new commit in the submodule (are there > cases where "git checkout other_branch" in the superproject does not > want to touch the submodule working tree?). > > There are cases where it is not possible; checking out the new > commit in the submodule working tree may not succeed due to local > modifications. But is that kind of complication limited to the > merge resolution case? Isn't it shared with normal "switching > branches" case as well? > > If so, it could be that your "git mergetool --theirs -- submodule" > is working around a wrong problem, and the right solution may be to > make "git checkout --theirs -- $path" to always do an appropriate > thing regardless of what kind of object $path is, no? True. Admittedly, I'm not a heavy submodule user myself so I tried crafting the directory vs submodule conflict to see what the usability is like. checkout --theirs and --ours could learn a few tricks. When trying to choose the directory in that situation I had to had to "git rm --cached" the submodule path so that git would recognize that there was no longer a conflict. That makes sense to me because I was following along by reading the mergetool code, but I don't think most users would know to "git rm" a path which they intend to keep. Afterwards, the .git file is left behind, which could cause problems elsewhere since we really don't want a .git file in that situation. I'm not even sure what to do about the .gitmodules file either. Here's the script I was using to setup the merge conflict in case anyone wants to get a feel for the usability around this edge case. Pass --submodule if you want the remote side to have a submodule. By default, the local side has a submodule and the remote side of the merge brings along a directory. That said, this really isn't an issue, per say. I first poked at it because I noticed that mergetool still needed stdin for some things. It's certainly an edge case, and perhaps this just shows that mergetool really is the right porcelain for the job when a user runs into these types of conflicts (the stdin thing really isn't an issue). #!/bin/sh if test "$1" = "--submodule" then first=with-directory second=with-submodule echo "local will be a directory, remote will be a submodule" else first=with-submodule second=with-directory echo "local will be a submodule, remote will be a directory" fi repo=submodule-conflict-$$ && echo "creating $repo" && mkdir "$repo" && cd "$repo" && git init && git commit --allow-empty -m'initial' && git checkout -b with-directory master && mkdir the-conflict && touch the-conflict/path && git add the-conflict/path && git commit -m'added path into the-conflict' && git checkout -b with-submodule master && git submodule add ./ the-conflict && git commit -m'added submodule as the-conflict' && git checkout -b tmp-merge master && git merge $first && git merge $second -- 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: Re*: mergetool: support --tool-help option like difftool does
David Aguilar writes: >> After hinting a low-hanging-fruit that would be an easy way for new >> people to dip their toes, I saw no takers for one month, so I ended >> up doing it myself. > > My bad,... Not yours. These hints I drop from time to time are meant for eager new people who want to dip their toes to the development (we don't do "assignments" but these are the closest that we have); you no longer quite qualify as "new" ;-) > While on the mergetool topic... Now we are not talking about "dip their toes" low hanging fruit anymore ;-) > Would the ability to resolve the various merge situations using > the command-line be a wanted addition? > > This would let a submodule or deleted/modified encountering > user do something like: > > $ git mergetool --theirs -- submodule > > ...and not have to remember the various git commands that it runs. Does it have to run various git commands? For a normal path, all it needs to do is "git checkout --theirs $path", no? What does it mean to resolve a conflicted merge of a gitlink to take "theirs"? We obviously would want to point the resolved gitlink at the submodule commit their side wants in the resulting index but what, if any, should we do to the submodule itself? Stepping back a bit, if there is no conflict, and as a result of a clean merge (this applies to the case where you check out another branch that has different commit at the submodule path), if gitlink changed to point at a different commit in the submodule, what should happen? If you start from a clean working tree, with your gitlink pointing at the commit that matches HEAD in the submodule, and if the working tree of the submodule does not have any local modification, it may be ideal to check out the new commit in the submodule (are there cases where "git checkout other_branch" in the superproject does not want to touch the submodule working tree?). There are cases where it is not possible; checking out the new commit in the submodule working tree may not succeed due to local modifications. But is that kind of complication limited to the merge resolution case? Isn't it shared with normal "switching branches" case as well? If so, it could be that your "git mergetool --theirs -- submodule" is working around a wrong problem, and the right solution may be to make "git checkout --theirs -- $path" to always do an appropriate thing regardless of what kind of object $path is, no? -- 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: Re*: mergetool: support --tool-help option like difftool does
On Wed, Aug 22, 2012 at 10:33 PM, Junio C Hamano wrote: > Junio C Hamano writes: > >> This way we do not have to risk the list of tools go out of sync >> between the implementation and the documentation. >> >> Signed-off-by: Junio C Hamano >> --- >> Junio C Hamano writes: >> +--tool-help:: + List the supported and available diff tools. >>> >>> This part is a good addition (but it already is mentioned in the >>> description of --tool above, so it is more or less a "Meh"). >> >> I noticed that the main while loop has style violations in its >> case/esac, but I left it as-is. Reindenting it would be a low >> hanging fruit janitor patch that would be a separate topic. > > After hinting a low-hanging-fruit that would be an easy way for new > people to dip their toes, I saw no takers for one month, so I ended > up doing it myself. My bad, I didn't catch this and should have, especially so because I had started on style fixing mergetool last week but ditched it; I assumed it would be unwanted. I will read more carefully. Just please don't tell Charles about it ;-) (I wanted to do this a long ago but at the time we didn't have the style guide for shell, and the time was perhaps not right...) I am of course in favor of this patch. While on the mergetool topic... Would the ability to resolve the various merge situations using the command-line be a wanted addition? This would let a submodule or deleted/modified encountering user do something like: $ git mergetool --theirs -- submodule ...and not have to remember the various git commands that it runs. This could touch just the parts of mergetool that require stdin, but probably should work for everything. It seems like exposing the guts of some of these mergetool functions via command-line flags could be helpful, but I'm not sure if that's overloading mergetool with too many features. What do you think? -- 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*: mergetool: support --tool-help option like difftool does
Junio C Hamano writes:
> This way we do not have to risk the list of tools go out of sync
> between the implementation and the documentation.
>
> Signed-off-by: Junio C Hamano
> ---
> Junio C Hamano writes:
>
>>> +--tool-help::
>>> + List the supported and available diff tools.
>>
>> This part is a good addition (but it already is mentioned in the
>> description of --tool above, so it is more or less a "Meh").
>
> I noticed that the main while loop has style violations in its
> case/esac, but I left it as-is. Reindenting it would be a low
> hanging fruit janitor patch that would be a separate topic.
After hinting a low-hanging-fruit that would be an easy way for new
people to dip their toes, I saw no takers for one month, so I ended
up doing it myself.
-- >8 --
Subject: mergetool: style fixes
This script is one of the sizeable ones that tempted people to copy
its "neibouring style" in their new code, but was littered with
styles incompatible with our style guide.
- use one tab, not four spaces, per indent level;
- long lines can be wrapped after '|', '&&', or '||' for
readability.
- structures like "if .. then .. else .. fi", "while .. do .. done"
are split into lines in such a way that does not require
unnecessary semicolon.
- case, esac and case-arms align at the same column.
Signed-off-by: Junio C Hamano
---
* The change would be easier to see if the reader runs these
command before and after applying this patch:
$ git diff -w git-mergetool.sh
$ git grep -e '^[\t]* ' -e '; *then' -e '; do' git-mergetool.sh
git-mergetool.sh | 581 +--
1 file changed, 308 insertions(+), 273 deletions(-)
diff --git i/git-mergetool.sh w/git-mergetool.sh
index 0db0c44..c50e18a 100755
--- i/git-mergetool.sh
+++ w/git-mergetool.sh
@@ -18,270 +18,301 @@ require_work_tree
# Returns true if the mode reflects a symlink
is_symlink () {
-test "$1" = 12
+ test "$1" = 12
}
is_submodule () {
-test "$1" = 16
+ test "$1" = 16
}
local_present () {
-test -n "$local_mode"
+ test -n "$local_mode"
}
remote_present () {
-test -n "$remote_mode"
+ test -n "$remote_mode"
}
base_present () {
-test -n "$base_mode"
+ test -n "$base_mode"
}
cleanup_temp_files () {
-if test "$1" = --save-backup ; then
- rm -rf -- "$MERGED.orig"
- test -e "$BACKUP" && mv -- "$BACKUP" "$MERGED.orig"
- rm -f -- "$LOCAL" "$REMOTE" "$BASE"
-else
- rm -f -- "$LOCAL" "$REMOTE" "$BASE" "$BACKUP"
-fi
+ if test "$1" = --save-backup
+ then
+ rm -rf -- "$MERGED.orig"
+ test -e "$BACKUP" && mv -- "$BACKUP" "$MERGED.orig"
+ rm -f -- "$LOCAL" "$REMOTE" "$BASE"
+ else
+ rm -f -- "$LOCAL" "$REMOTE" "$BASE" "$BACKUP"
+ fi
}
describe_file () {
-mode="$1"
-branch="$2"
-file="$3"
-
-printf " {%s}: " "$branch"
-if test -z "$mode"; then
- echo "deleted"
-elif is_symlink "$mode" ; then
- echo "a symbolic link -> '$(cat "$file")'"
-elif is_submodule "$mode" ; then
- echo "submodule commit $file"
-else
- if base_present; then
- echo "modified file"
+ mode="$1"
+ branch="$2"
+ file="$3"
+
+ printf " {%s}: " "$branch"
+ if test -z "$mode"
+ then
+ echo "deleted"
+ elif is_symlink "$mode"
+ then
+ echo "a symbolic link -> '$(cat "$file")'"
+ elif is_submodule "$mode"
+ then
+ echo "submodule commit $file"
+ elif base_present
+ then
+ echo "modified file"
else
- echo "created file"
+ echo "created file"
fi
-fi
}
-
resolve_symlink_merge () {
-while true; do
- printf "Use (l)ocal or (r)emote, or (a)bort? "
- read ans || return 1
- case "$ans" in
- [lL]*)
- git checkout-index -f --stage=2 -- "$MERGED"
- git add -- "$MERGED"
- cleanup_temp_files --save-backup
- return 0
- ;;
- [rR]*)
- git checkout-index -f --stage=3 -- "$MERGED"
- git add -- "$MERGED"
- cleanup_temp_files --save-backup
- return 0
- ;;
- [aA]*)
- return 1
- ;;
- esac
+ while true
+ do
+ printf "Use (l)ocal or (r)emote, or (a)bort? "
+ read ans || return 1
+ case "$ans" in
+ [lL]*)
+ git checkout-index -f --stage=2 -- "$MERGED"
+ git add -- "$MERGED"
+ cleanup_temp_files --save-backup
+ return 0
+ ;;
+ [rR]*)
+ git checkout-index -f --stage=3 -- "$MERGED"
+
Re: [PATCH] mergetool: support --tool-help option like difftool does
Sebastian Schuberth writes: > On Mon, Jul 23, 2012 at 11:16 PM, Junio C Hamano wrote: > >> There are only five or six classes of environment that matter in the >> real world for the purpose of giving "well known" examples: Windows, >> MacOS X, Gnome, KDE and Linux terminal. By picking a representative >> one from each and listing them, the end result would have at least >> one that people from various platforms have _heard of_ and can guess >> what they do. The "most common" is secondary, and "well known" is > > I completely agree with this. So we should take the chance and add a > Windows representative to the list of difftools, no? I do not care very deeply either way. I am more interested in seeing --tool-list options supported by both so that we can get rid of hardcoded list from the completion script. > This drops the explicit mention of --tool-help as an option in the > documentation compared to my patch. Do you want to keep --tool-help > being mentioned inline as part of the --tool option documentation > only? While I do not think having one would hurt that much, I do not think a separate entry would add much value either, so I chose not to clutter the documentation further. The "--tool=" entry is enough hint to draw eyes of readers who want to find out what kind of backends are supported, and --tool-help is mentioned there quite prominently. -- 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] mergetool: support --tool-help option like difftool does
On Mon, Jul 23, 2012 at 11:16 PM, Junio C Hamano wrote: > There are only five or six classes of environment that matter in the > real world for the purpose of giving "well known" examples: Windows, > MacOS X, Gnome, KDE and Linux terminal. By picking a representative > one from each and listing them, the end result would have at least > one that people from various platforms have _heard of_ and can guess > what they do. The "most common" is secondary, and "well known" is I completely agree with this. So we should take the chance and add a Windows representative to the list of difftools, no? > Unlike POSIXy folks, where IRIX or Solaris users are likely to have > heard of Gnome tools even if they do not use the environment on > their platforms, Windows users tend to be isolated bunch, so it > would not hurt to include at least one well-known Windows-only tool > in the list. Heh. I believe POSIX folks are no less isolated. (How many Windows-only tools would you recognize by name?) They're just isolated in a bigger world ;-) > Here is a v2, with documentation updates. This drops the explicit mention of --tool-help as an option in the documentation compared to my patch. Do you want to keep --tool-help being mentioned inline as part of the --tool option documentation only? -- Sebastian Schuberth -- 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] mergetool: support --tool-help option like difftool does
David Aguilar writes:
> On Mon, Jul 23, 2012 at 1:14 PM, Sebastian Schuberth
> wrote:
>> On Mon, Jul 23, 2012 at 9:52 PM, Junio C Hamano wrote:
>>
>>>> -t ::
>>>> --tool=::
>>>> - Use the diff tool specified by . Valid values include
>>>> - emerge, kompare, meld, and vimdiff. Run `git difftool --tool-help`
>>>> - for the list of valid settings.
>>>> + Use the diff tool specified by .
>>>
>>> I do not see how it is an improvement to drop the most common ones.
>>> People sometimes read documentation without having an access to
>>> shell to run "cmd --tool-help", and a list of handful of well known
>>> ones would serve as a good hint to let the reader know the kind of
>>> commands the front-end is capable of spawning, which in turn help
>>> such a reader to imagine how the command is used to judge if it is
>>> something the reader wants to use.
>>
>> I don't agree. What "most common ones" are depends on your platform
>> and is sort of subjective. So it should be either all or non here.
>
> Let's please leave this section as-is.
Yes.
There are only five or six classes of environment that matter in the
real world for the purpose of giving "well known" examples: Windows,
MacOS X, Gnome, KDE and Linux terminal. By picking a representative
one from each and listing them, the end result would have at least
one that people from various platforms have _heard of_ and can guess
what they do. The "most common" is secondary, and "well known" is
the keyword. "Can guess what they do" is the point of the phrase
"Valid values _include_". Even if you are hard-core Emacs user, it
is likely that you've heard of vimdiff---and in that case, including
vimdiff would be enough. Likewise for showing kompare to Gnome users.
Unlike POSIXy folks, where IRIX or Solaris users are likely to have
heard of Gnome tools even if they do not use the environment on
their platforms, Windows users tend to be isolated bunch, so it
would not hurt to include at least one well-known Windows-only tool
in the list.
> Yes, indeed, it is arbitrary. It does have some merit, though--it is
> also a good compromise between unhelpful (listing nothing) and painful
> to maintain (listing everything).
Here is a v2, with documentation updates.
-- >8 --
Subject: [PATCH] mergetool: support --tool-help option like difftool does
This way we do not have to risk the list of tools going out of sync
between the implementation and the documentation.
In the same spirit as bf73fc2 (difftool: print list of valid tools
with '--tool-help', 2012-03-29), trim the list of merge backends in
the documentation. We do not want to have a complete of valid tools
there; we only want a list to help people guess what kind of things
the tools that can be specified there, and refer them to --tool-help
for a complete list.
Signed-off-by: Junio C Hamano
---
Documentation/git-mergetool.txt | 6 +++---
git-mergetool--lib.sh | 6 +-
git-mergetool.sh| 42 -
3 files changed, 49 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 2a49de7..d1e08d2 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -27,9 +27,9 @@ OPTIONS
-t ::
--tool=::
Use the merge resolution program specified by .
- Valid merge tools are:
- araxis, bc3, diffuse, ecmerge, emerge, gvimdiff, kdiff3,
- meld, opendiff, p4merge, tkdiff, tortoisemerge, vimdiff and xxdiff.
+ Valid values include emerge, gvimdiff, kdiff3,
+ meld, vimdiff, and tortoisemerge. Run `git mergetool --tool-help`
+ for the list of valid settings.
+
If a merge resolution program is not specified, 'git mergetool'
will use the configuration variable `merge.tool`. If the
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index ed630b2..f730253 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -111,7 +111,7 @@ run_merge_tool () {
return $status
}
-guess_merge_tool () {
+list_merge_tool_candidates () {
if merge_mode
then
tools="tortoisemerge"
@@ -136,6 +136,10 @@ guess_merge_tool () {
tools="$tools emerge vimdiff"
;;
esac
+}
+
+guess_merge_tool () {
+ list_merge_tool_candidates
echo >&2 "merge tool candidates: $tools"
# Loop over each candidate and stop when a valid merge tool is found.
diff --git a/git-mergetool.sh b/git-mergetool.sh
index a9f23f7..0db0c44 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -8,7 +8,7 @@
# at the di
Re: [PATCH] mergetool: support --tool-help option like difftool does
On Mon, Jul 23, 2012 at 1:14 PM, Sebastian Schuberth wrote: > On Mon, Jul 23, 2012 at 9:52 PM, Junio C Hamano wrote: > >>> -t :: >>> --tool=:: >>> - Use the diff tool specified by . Valid values include >>> - emerge, kompare, meld, and vimdiff. Run `git difftool --tool-help` >>> - for the list of valid settings. >>> + Use the diff tool specified by . >> >> I do not see how it is an improvement to drop the most common ones. >> People sometimes read documentation without having an access to >> shell to run "cmd --tool-help", and a list of handful of well known >> ones would serve as a good hint to let the reader know the kind of >> commands the front-end is capable of spawning, which in turn help >> such a reader to imagine how the command is used to judge if it is >> something the reader wants to use. > > I don't agree. What "most common ones" are depends on your platform > and is sort of subjective. So it should be either all or non here. Let's please leave this section as-is. This part of the documentation has had a fair amount of churn. Specifically, it would get touched every time a new tool was added. The point of bf73fc212a159210398b6d46ed5e9101c650e7db was to change it *one last time* into something that is helpful, but not a substitute for the real list output by --tool-help. If any changes are done then it should be to make git-mergetool.txt match the advice given in git-difftool.txt. > Your argument about people not having shell access is a valid one, but > still that would mean to list all tools in my opinion. And listing all > tools again thwarts our goal to reduce the number of places where new > merge / diff tools need to be added. For people adding new merge / > diff tools it is just clearer what places need to be modified if there > are no places that list an arbitrary subset of tools. Yes, indeed, it is arbitrary. It does have some merit, though--it is also a good compromise between unhelpful (listing nothing) and painful to maintain (listing everything). -- 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] mergetool: support --tool-help option like difftool does
On Mon, Jul 23, 2012 at 9:52 PM, Junio C Hamano wrote: >> -t :: >> --tool=:: >> - Use the diff tool specified by . Valid values include >> - emerge, kompare, meld, and vimdiff. Run `git difftool --tool-help` >> - for the list of valid settings. >> + Use the diff tool specified by . > > I do not see how it is an improvement to drop the most common ones. > People sometimes read documentation without having an access to > shell to run "cmd --tool-help", and a list of handful of well known > ones would serve as a good hint to let the reader know the kind of > commands the front-end is capable of spawning, which in turn help > such a reader to imagine how the command is used to judge if it is > something the reader wants to use. I don't agree. What "most common ones" are depends on your platform and is sort of subjective. So it should be either all or non here. Your argument about people not having shell access is a valid one, but still that would mean to list all tools in my opinion. And listing all tools again thwarts our goal to reduce the number of places where new merge / diff tools need to be added. For people adding new merge / diff tools it is just clearer what places need to be modified if there are no places that list an arbitrary subset of tools. -- Sebastian Schuberth -- 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] mergetool: support --tool-help option like difftool does
Sebastian Schuberth writes:
> This way we do not have to risk the list of tools go out of sync
> between the implementation and the documentation. Adjust the documentation
> accordingly to not explicitly list the tools but refer to --tool-help.
>
> Signed-off-by: Junio C Hamano
> Signed-off-by: Sebastian Schuberth
> ---
> Documentation/git-difftool.txt | 7 ---
> Documentation/git-mergetool.txt | 8
> git-mergetool--lib.sh | 11 ++-
> git-mergetool.sh| 42
> -
> 4 files changed, 59 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
> index 31fc2e3..0bdfe35 100644
> --- a/Documentation/git-difftool.txt
> +++ b/Documentation/git-difftool.txt
> @@ -36,9 +36,10 @@ OPTIONS
>
> -t ::
> --tool=::
> - Use the diff tool specified by . Valid values include
> - emerge, kompare, meld, and vimdiff. Run `git difftool --tool-help`
> - for the list of valid settings.
> + Use the diff tool specified by .
I do not see how it is an improvement to drop the most common ones.
People sometimes read documentation without having an access to
shell to run "cmd --tool-help", and a list of handful of well known
ones would serve as a good hint to let the reader know the kind of
commands the front-end is capable of spawning, which in turn help
such a reader to imagine how the command is used to judge if it is
something the reader wants to use.
> +
> +--tool-help::
> + List all supported values for .
> +
> If a diff tool is not specified, 'git difftool'
> will use the configuration variable `diff.tool`. If the
> diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
> index 2a49de7..99e53b1 100644
> --- a/Documentation/git-mergetool.txt
> +++ b/Documentation/git-mergetool.txt
> @@ -26,10 +26,10 @@ OPTIONS
> ---
> -t ::
> --tool=::
> - Use the merge resolution program specified by .
> - Valid merge tools are:
> - araxis, bc3, diffuse, ecmerge, emerge, gvimdiff, kdiff3,
> - meld, opendiff, p4merge, tkdiff, tortoisemerge, vimdiff and xxdiff.
> + Use the merge tool specified by .
Likewise. Dropping araxis, bc3, etc. to trim the list to 4-5 items
that people would know what they are would make sense, though. The
purpose of the list is not to tell if the reader's favorite tool is
supported or not---it is to hint the flavor of what kind of things
the command spawned would be capable of.
> +
> +--tool-help::
> + List all supported values for .
> +
> If a merge resolution program is not specified, 'git mergetool'
> will use the configuration variable `merge.tool`. If the
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index ed630b2..f0865d4 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -111,15 +111,18 @@ run_merge_tool () {
> return $status
> }
>
> -guess_merge_tool () {
> +list_merge_tool_candidates () {
> + # Add tools that can either do merging or diffing, but not both.
> if merge_mode
> then
> tools="tortoisemerge"
> else
> tools="kompare"
> fi
> +
> if test -n "$DISPLAY"
> then
> + # Prefer GTK-based tools under Gnome.
> if test -n "$GNOME_DESKTOP_SESSION_ID"
> then
> tools="meld opendiff kdiff3 tkdiff xxdiff $tools"
> @@ -128,6 +131,8 @@ guess_merge_tool () {
> fi
> tools="$tools gvimdiff diffuse ecmerge p4merge araxis bc3"
> fi
> +
> + # Prefer vimdiff if vim is the default editor.
> case "${VISUAL:-$EDITOR}" in
> *vim*)
> tools="$tools vimdiff emerge"
> @@ -136,6 +141,10 @@ guess_merge_tool () {
> tools="$tools emerge vimdiff"
> ;;
> esac
> +}
> +
> +guess_merge_tool () {
> + list_merge_tool_candidates
> echo >&2 "merge tool candidates: $tools"
>
> # Loop over each candidate and stop when a valid merge tool is found.
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index a9f23f7..0db0c44 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -8,7 +8,7 @@
> # at the discretion of Junio C Hamano.
> #
>
> -USAGE='[--tool=tool] [-y|--no-prompt|--prompt] [file to merge] ...'
> +USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge]
> ...'
> SUBDIRECTORY_OK=Yes
> OPTIONS_SPEC=
> TOOL_MODE=merge
> @@ -284,11 +284,51 @@ merge_file () {
> return 0
> }
>
> +show_tool_help () {
> + TOOL_MODE=merge
> + list_merge_tool_candidates
> + unavailable= available= LF='
> +'
> + for i in $tools
> + do
> + merge_tool_path=$(translate_merge_tool_path "$i")
> + if type "$merge_tool_path" >/dev/null 2>&1
> + then
> + available="$available$i$LF"
> + else
> + unavailable="$
[PATCH] mergetool: support --tool-help option like difftool does
This way we do not have to risk the list of tools go out of sync
between the implementation and the documentation. Adjust the documentation
accordingly to not explicitly list the tools but refer to --tool-help.
Signed-off-by: Junio C Hamano
Signed-off-by: Sebastian Schuberth
---
Documentation/git-difftool.txt | 7 ---
Documentation/git-mergetool.txt | 8
git-mergetool--lib.sh | 11 ++-
git-mergetool.sh| 42 -
4 files changed, 59 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
index 31fc2e3..0bdfe35 100644
--- a/Documentation/git-difftool.txt
+++ b/Documentation/git-difftool.txt
@@ -36,9 +36,10 @@ OPTIONS
-t ::
--tool=::
- Use the diff tool specified by . Valid values include
- emerge, kompare, meld, and vimdiff. Run `git difftool --tool-help`
- for the list of valid settings.
+ Use the diff tool specified by .
+
+--tool-help::
+ List all supported values for .
+
If a diff tool is not specified, 'git difftool'
will use the configuration variable `diff.tool`. If the
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 2a49de7..99e53b1 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -26,10 +26,10 @@ OPTIONS
---
-t ::
--tool=::
- Use the merge resolution program specified by .
- Valid merge tools are:
- araxis, bc3, diffuse, ecmerge, emerge, gvimdiff, kdiff3,
- meld, opendiff, p4merge, tkdiff, tortoisemerge, vimdiff and xxdiff.
+ Use the merge tool specified by .
+
+--tool-help::
+ List all supported values for .
+
If a merge resolution program is not specified, 'git mergetool'
will use the configuration variable `merge.tool`. If the
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index ed630b2..f0865d4 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -111,15 +111,18 @@ run_merge_tool () {
return $status
}
-guess_merge_tool () {
+list_merge_tool_candidates () {
+ # Add tools that can either do merging or diffing, but not both.
if merge_mode
then
tools="tortoisemerge"
else
tools="kompare"
fi
+
if test -n "$DISPLAY"
then
+ # Prefer GTK-based tools under Gnome.
if test -n "$GNOME_DESKTOP_SESSION_ID"
then
tools="meld opendiff kdiff3 tkdiff xxdiff $tools"
@@ -128,6 +131,8 @@ guess_merge_tool () {
fi
tools="$tools gvimdiff diffuse ecmerge p4merge araxis bc3"
fi
+
+ # Prefer vimdiff if vim is the default editor.
case "${VISUAL:-$EDITOR}" in
*vim*)
tools="$tools vimdiff emerge"
@@ -136,6 +141,10 @@ guess_merge_tool () {
tools="$tools emerge vimdiff"
;;
esac
+}
+
+guess_merge_tool () {
+ list_merge_tool_candidates
echo >&2 "merge tool candidates: $tools"
# Loop over each candidate and stop when a valid merge tool is found.
diff --git a/git-mergetool.sh b/git-mergetool.sh
index a9f23f7..0db0c44 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -8,7 +8,7 @@
# at the discretion of Junio C Hamano.
#
-USAGE='[--tool=tool] [-y|--no-prompt|--prompt] [file to merge] ...'
+USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge]
...'
SUBDIRECTORY_OK=Yes
OPTIONS_SPEC=
TOOL_MODE=merge
@@ -284,11 +284,51 @@ merge_file () {
return 0
}
+show_tool_help () {
+ TOOL_MODE=merge
+ list_merge_tool_candidates
+ unavailable= available= LF='
+'
+ for i in $tools
+ do
+ merge_tool_path=$(translate_merge_tool_path "$i")
+ if type "$merge_tool_path" >/dev/null 2>&1
+ then
+ available="$available$i$LF"
+ else
+ unavailable="$unavailable$i$LF"
+ fi
+ done
+ if test -n "$available"
+ then
+ echo "'git mergetool --tool=' may be set to one of the
following:"
+ echo "$available" | sort | sed -e 's/^/ /'
+ else
+ echo "No suitable tool for 'git mergetool --tool=' found."
+ fi
+ if test -n "$unavailable"
+ then
+ echo
+ echo 'The following tools are valid, but not currently
available:'
+ echo "$unavailable" | sort | sed -e 's/^/ /'
+ fi
+ if test -n "$unavailable$available"
+ then
+ echo
+ echo "Some of the tools listed above only work in a windowed"
+ echo "environment. If run in a terminal-only session, they will
fail."
+ fi
+ exit 0
+}
+
prompt=$(git config --bool mergetool.prompt || echo true)
while test $# != 0
do
ca
Re: mergetool: support --tool-help option like difftool does
On 23.07.2012 19:21, Junio C Hamano wrote: > This way we do not have to risk the list of tools go out of sync > between the implementation and the documentation. > > Signed-off-by: Junio C Hamano Thanks! I've squashed this with [PATCH v2 5/5] Add a few more code comments and blank lines in guess_merge_tool and parts of [PATCH 3/5] Explicitly list all valid diff tools and document --tool-help as an option and adjusted the docs for git-mergetool accordingly. -- Sebastian Schuberth -- 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
mergetool: support --tool-help option like difftool does
This way we do not have to risk the list of tools go out of sync
between the implementation and the documentation.
Signed-off-by: Junio C Hamano
---
Junio C Hamano writes:
>> +--tool-help::
>> +List the supported and available diff tools.
>
> This part is a good addition (but it already is mentioned in the
> description of --tool above, so it is more or less a "Meh").
I noticed that the main while loop has style violations in its
case/esac, but I left it as-is. Reindenting it would be a low
hanging fruit janitor patch that would be a separate topic.
git-mergetool--lib.sh | 6 +-
git-mergetool.sh | 42 +-
2 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index ed630b2..f730253 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -111,7 +111,7 @@ run_merge_tool () {
return $status
}
-guess_merge_tool () {
+list_merge_tool_candidates () {
if merge_mode
then
tools="tortoisemerge"
@@ -136,6 +136,10 @@ guess_merge_tool () {
tools="$tools emerge vimdiff"
;;
esac
+}
+
+guess_merge_tool () {
+ list_merge_tool_candidates
echo >&2 "merge tool candidates: $tools"
# Loop over each candidate and stop when a valid merge tool is found.
diff --git a/git-mergetool.sh b/git-mergetool.sh
index a9f23f7..0db0c44 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -8,7 +8,7 @@
# at the discretion of Junio C Hamano.
#
-USAGE='[--tool=tool] [-y|--no-prompt|--prompt] [file to merge] ...'
+USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge]
...'
SUBDIRECTORY_OK=Yes
OPTIONS_SPEC=
TOOL_MODE=merge
@@ -284,11 +284,51 @@ merge_file () {
return 0
}
+show_tool_help () {
+ TOOL_MODE=merge
+ list_merge_tool_candidates
+ unavailable= available= LF='
+'
+ for i in $tools
+ do
+ merge_tool_path=$(translate_merge_tool_path "$i")
+ if type "$merge_tool_path" >/dev/null 2>&1
+ then
+ available="$available$i$LF"
+ else
+ unavailable="$unavailable$i$LF"
+ fi
+ done
+ if test -n "$available"
+ then
+ echo "'git mergetool --tool=' may be set to one of the
following:"
+ echo "$available" | sort | sed -e 's/^/ /'
+ else
+ echo "No suitable tool for 'git mergetool --tool=' found."
+ fi
+ if test -n "$unavailable"
+ then
+ echo
+ echo 'The following tools are valid, but not currently
available:'
+ echo "$unavailable" | sort | sed -e 's/^/ /'
+ fi
+ if test -n "$unavailable$available"
+ then
+ echo
+ echo "Some of the tools listed above only work in a windowed"
+ echo "environment. If run in a terminal-only session, they will
fail."
+ fi
+ exit 0
+}
+
prompt=$(git config --bool mergetool.prompt || echo true)
while test $# != 0
do
case "$1" in
+ --tool-help)
+ show_tool_help
+ ;;
-t|--tool*)
case "$#,$1" in
*,*=*)
--
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
