Re: [PATCH] Remove --no-gui option from difftool usage string
On Fri, Feb 03, 2017 at 09:58:09PM -0800, Jacob Keller wrote: > On Fri, Feb 3, 2017 at 6:56 PM, Denton Liu <liu.den...@gmail.com> wrote: > > The --no-gui option not documented in the manpage, nor is it actually > > used in the source code. This change removes it from the usage help > > that's printed. > > > > Signed-off-by: Denton Liu <liu.den...@gmail.com> > > --- > > git-difftool.perl | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/git-difftool.perl b/git-difftool.perl > > index a5790d03a..657d5622d 100755 > > --- a/git-difftool.perl > > +++ b/git-difftool.perl > > @@ -29,8 +29,8 @@ sub usage > > print << 'USAGE'; > > usage: git difftool [-t|--tool=] [--tool-help] > > [-x|--extcmd=] > > -[-g|--gui] [--no-gui] > > -[--prompt] [-y|--no-prompt] > > +[-g|--gui] [--prompt] > > +[-y|--no-prompt] > > [-d|--dir-diff] > > ['git diff' options] > > USAGE > > -- > > 2.11.0 > > > > Aren't "--no-foo" options automatically created for booleans when > using our option parsing code? > > Thanks, > Jake Sorry, I guess I didn't notice that. Would it make sense to document it in the manpage then? -- Denton Liu
[PATCH] Remove --no-gui option from difftool usage string
The --no-gui option not documented in the manpage, nor is it actually used in the source code. This change removes it from the usage help that's printed. Signed-off-by: Denton Liu <liu.den...@gmail.com> --- git-difftool.perl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git-difftool.perl b/git-difftool.perl index a5790d03a..657d5622d 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -29,8 +29,8 @@ sub usage print << 'USAGE'; usage: git difftool [-t|--tool=] [--tool-help] [-x|--extcmd=] -[-g|--gui] [--no-gui] -[--prompt] [-y|--no-prompt] +[-g|--gui] [--prompt] +[-y|--no-prompt] [-d|--dir-diff] ['git diff' options] USAGE -- 2.11.0
[PATCH v2] Document the --no-gui option in difftool
Prior to this, the `--no-gui` option was not documented in the manpage. This commit introduces this into the manpage Signed-off-by: Denton Liu <liu.den...@gmail.com> --- Documentation/git-difftool.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index 224fb3090..96c26e6aa 100644 --- a/Documentation/git-difftool.txt +++ b/Documentation/git-difftool.txt @@ -86,10 +86,11 @@ instead. `--no-symlinks` is the default on Windows. Additionally, `$BASE` is set in the environment. -g:: ---gui:: +--[no-]gui:: When 'git-difftool' is invoked with the `-g` or `--gui` option the default diff tool will be read from the configured - `diff.guitool` variable instead of `diff.tool`. + `diff.guitool` variable instead of `diff.tool`. The `--no-gui` + option can be used to override this setting. --[no-]trust-exit-code:: 'git-difftool' invokes a diff tool individually on each file. -- 2.11.1
[PATCH] Add --gui option to mergetool
* fix the discrepancy between difftool and mergetool where
the former has the --gui flag and the latter does not by adding the
functionality to mergetool
* make difftool read 'merge.guitool' as a fallback, in accordance to the
manpage for difftool: "git difftool falls back to git mergetool
config variables when the difftool equivalents have not been defined"
* add guitool-related completions
Signed-off-by: Denton Liu <liu.den...@gmail.com>
---
Documentation/git-difftool.txt | 3 ++-
Documentation/git-mergetool.txt| 8 +++-
contrib/completion/git-completion.bash | 6 --
git-mergetool.sh | 5 -
4 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
index 224fb3090..0b5d29237 100644
--- a/Documentation/git-difftool.txt
+++ b/Documentation/git-difftool.txt
@@ -89,7 +89,8 @@ instead. `--no-symlinks` is the default on Windows.
--gui::
When 'git-difftool' is invoked with the `-g` or `--gui` option
the default diff tool will be read from the configured
- `diff.guitool` variable instead of `diff.tool`.
+ `diff.guitool` variable instead of `diff.tool`. If `diff.guitool`
+ is not defined, it will try and read from `merge.guitool`.
--[no-]trust-exit-code::
'git-difftool' invokes a diff tool individually on each file.
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 3622d6648..2ab56efcf 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -8,7 +8,7 @@ git-mergetool - Run merge conflict resolution tools to resolve
merge conflicts
SYNOPSIS
[verse]
-'git mergetool' [--tool=] [-y | --[no-]prompt] [...]
+'git mergetool' [--tool=] [-g|--gui] [-y | --[no-]prompt] [...]
DESCRIPTION
---
@@ -64,6 +64,12 @@ variable `mergetool..trustExitCode` can be set to
`true`.
Otherwise, 'git mergetool' will prompt the user to indicate the
success of the resolution after the custom tool has exited.
+-g::
+--gui::
+ When 'git-mergetool' is invoked with the `-g` or `--gui` option
+ the default diff tool will be read from the configured
+ `merge.guitool` variable instead of `merge.tool`.
+
--tool-help::
Print a list of merge tools that may be used with `--tool`.
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 21016bf8d..8a7427f3c 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1268,7 +1268,7 @@ _git_difftool ()
--base --ours --theirs
--no-renames --diff-filter= --find-copies-harder
--relative --ignore-submodules
- --tool="
+ --tool= --gui"
return
;;
esac
@@ -1566,7 +1566,7 @@ _git_mergetool ()
return
;;
--*)
- __gitcomp "--tool="
+ __gitcomp "--tool= --gui"
return
;;
esac
@@ -2189,6 +2189,7 @@ _git_config ()
diff.submodule
diff.suppressBlankEmpty
diff.tool
+ diff.guitool
diff.wordRegex
diff.algorithm
difftool.
@@ -2290,6 +2291,7 @@ _git_config ()
merge.renormalize
merge.stat
merge.tool
+ merge.guitool
merge.verbosity
mergetool.
mergetool.keepBackup
diff --git a/git-mergetool.sh b/git-mergetool.sh
index e52b4e4f2..a17668752 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -9,7 +9,7 @@
# at the discretion of Junio C Hamano.
#
-USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [-O]
[file to merge] ...'
+USAGE='[--tool=tool] [-g|--gui] [--tool-help] [-y|--no-prompt|--prompt]
[-O] [file to merge] ...'
SUBDIRECTORY_OK=Yes
NONGIT_OK=Yes
OPTIONS_SPEC=
@@ -414,6 +414,9 @@ main () {
shift ;;
esac
;;
+ -g|--gui)
+ merge_tool=$(git config merge.guitool)
+ ;;
-y|--no-prompt)
prompt=false
;;
--
2.11.0.21.ga274e0a.dirty
[PATCH] Document the --no-gui option in difftool
Prior to this, the `--no-gui` option was not documented in the manpage. This commit introduces this into the manpage Signed-off-by: Denton Liu <liu.den...@gmail.com> --- Documentation/git-difftool.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index 224fb3090..a2661d9cc 100644 --- a/Documentation/git-difftool.txt +++ b/Documentation/git-difftool.txt @@ -87,9 +87,11 @@ instead. `--no-symlinks` is the default on Windows. -g:: --gui:: +--no-gui:: When 'git-difftool' is invoked with the `-g` or `--gui` option the default diff tool will be read from the configured - `diff.guitool` variable instead of `diff.tool`. + `diff.guitool` variable instead of `diff.tool`. The `--no-gui` + option can be used to override this setting. --[no-]trust-exit-code:: 'git-difftool' invokes a diff tool individually on each file. -- 2.12.0.rc0.208.g81c5d00b2
mergetool and difftool inconsistency?
Hello all, I was wondering if there is any reason why 'git difftool' accepts the '-g|--gui' whereas 'git mergetool' does not have an option to accept that flag. Please let me know if this is intentional, otherwise I can write up a patch to add the GUI flag to 'mergetool'. Thanks!
[PATCH] completion: complete git submodule subcommands
Allow git submodule subcommands to be completed. This allows the '--remote' in the command 'git submodule update --remote', for example, to be fully completed. Signed-off-by: Denton Liu <liu.den...@gmail.com> --- Hi Shawn, sorry this is my first contribution to a mailing-list based project. If I've done anything wrong, please let me know. Thanks, Denton Liu --- contrib/completion/git-completion.bash | 46 ++ 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 21016bf8d..941fbdfe2 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2556,17 +2556,41 @@ _git_submodule () __git_has_doubledash && return local subcommands="add status init deinit update summary foreach sync" - if [ -z "$(__git_find_on_cmdline "$subcommands")" ]; then - case "$cur" in - --*) - __gitcomp "--quiet --cached" - ;; - *) - __gitcomp "$subcommands" - ;; - esac - return - fi + local subcommand="$(__git_find_on_cmdline "$subcommands")" + + case "$subcommand,$cur" in + ,--*) + __gitcomp "--quiet" + ;; + ,*) + __gitcomp "$subcommands --quiet" + ;; + add,--*) + __gitcomp "--force --name --reference --depth" + ;; + status,--*) + __gitcomp "--cached --recursive" + ;; + deinit,--*) + __gitcomp "--force --all" + ;; + update,--*) + __gitcomp " + --init --remote --no-fetch --no-recommended-shallow + --recommended-shallow --force --rebase --merge --reference + --depth --recursive --jobs + " + ;; + summary,--*) + __gitcomp "--cached --files --summary-limit" + ;; + summary,*) + __gitcomp_nl "$(__git_refs)" + ;; + foreach,--*|sync,--*) + __gitcomp "--recursive" + ;; + esac } _git_svn () -- 2.11.0
[PATCH 2/2] completion: add bash completion for 'filter-branch'
Signed-off-by: Denton Liu <liu.den...@gmail.com>
---
contrib/completion/git-completion.bash | 17 +
1 file changed, 17 insertions(+)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 51832108e..b4cbea5c7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1297,6 +1297,23 @@ _git_fetch ()
__git_complete_remote_or_refspec
}
+__git_filter_branch_options="
+ --env-filter --tree-filter --index-filter --parent-filter --msg-filter
+ --commit-filter --tag-name-filter --subdirectory-filter --prune-empty
+ --original --force
+"
+_git_filter_branch ()
+{
+ __git_has_doubledash && __git_complete_rev_list_command && return
+
+ case "$cur" in
+ --*)
+ __gitcomp "$__git_filter_branch_options"
+ return
+ ;;
+ esac
+}
+
__git_format_patch_options="
--stdout --attach --no-attach --thread --thread= --no-thread
--numbered --start-number --numbered-files --keep-subject --signoff
--
2.11.0
[PATCH 1/2] completion: add bash completion for 'git rev-list'
Signed-off-by: Denton Liu <liu.den...@gmail.com>
---
contrib/completion/git-completion.bash | 30 ++
1 file changed, 30 insertions(+)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 21016bf8d..51832108e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2430,6 +2430,36 @@ _git_revert ()
__gitcomp_nl "$(__git_refs)"
}
+__git_rev_list_options="
+ --max-count= --skip= --max-age= --min-age= --sparse --merges --no-merges
+ --min-parents= --no-min-parents --max-parents= --no-max-parents --first-parent
+ --remove-empty --full-history --not --all --branches= --tags= --remotes=
+ --glob= --ignore-missing --stdin --quiet --topo-order --parents --timestamp
+ --left-right --left-only --right-only --cherry-mark --cherry-pick --encoding=
+ --author= --committer= --grep= --regexp-ignore-case --extended-regexp
+ --fixed-strings --date= --objects --objects-edge --objects-edge-aggressive
+ --unpacked --pretty --header --bisect --bisect-vars --bisect-all --merge
+ --reverse --walk-reflogs --no-walk --do-walk --count --use-bitmap-index
+"
+
+__git_complete_rev_list_command ()
+{
+ case "$cur" in
+ --*)
+ __gitcomp "$__git_rev_list_options"
+ return 0
+ ;;
+ esac
+ return 1
+}
+
+_git_rev_list ()
+{
+ __git_has_doubledash && return
+
+ __git_complete_rev_list_command || __gitcomp_nl "$(__git_refs)"
+}
+
_git_rm ()
{
case "$cur" in
--
2.11.0
[RFC PATCH 1/2] completion: add bash completion for 'git rev-list'
Signed-off-by: Denton Liu <liu.den...@gmail.com>
---
This patch isn't strictly necessary since 'git rev-list' isn't a porcelain
command. However, it might be nice to include in case users interactively call
'git rev-list' anyway.
---
contrib/completion/git-completion.bash | 30 ++
1 file changed, 30 insertions(+)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 41ee52991..412485369 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2499,6 +2499,36 @@ _git_revert ()
__gitcomp_nl "$(__git_refs)"
}
+__git_rev_list_options="
+ --max-count= --skip= --max-age= --min-age= --sparse --merges --no-merges
+ --min-parents= --no-min-parents --max-parents= --no-max-parents --first-parent
+ --remove-empty --full-history --not --all --branches= --tags= --remotes=
+ --glob= --ignore-missing --stdin --quiet --topo-order --parents --timestamp
+ --left-right --left-only --right-only --cherry-mark --cherry-pick --encoding=
+ --author= --committer= --grep= --regexp-ignore-case --extended-regexp
+ --fixed-strings --date= --objects --objects-edge --objects-edge-aggressive
+ --unpacked --pretty --header --bisect --bisect-vars --bisect-all --merge
+ --reverse --walk-reflogs --no-walk --do-walk --count --use-bitmap-index
+"
+
+__git_complete_rev_list_command ()
+{
+ case "$cur" in
+ --*)
+ __gitcomp "$__git_rev_list_options"
+ return 0
+ ;;
+ esac
+ return 1
+}
+
+_git_rev_list ()
+{
+ __git_has_doubledash && return
+
+ __git_complete_rev_list_command || __gitcomp_nl "$(__git_refs)"
+}
+
_git_rm ()
{
case "$cur" in
--
2.12.0.1.g5415fdfc5.dirty
[PATCH v2 5/5] Remove outdated info in difftool manpage
When difftool was rewritten in C, it removed the capability to read fallback configs from mergetool. This changes the documentation to reflect this. Signed-off-by: Denton Liu <liu.den...@gmail.com> --- Documentation/git-difftool.txt | 3 --- 1 file changed, 3 deletions(-) diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index 96c26e6aa..a00cb033e 100644 --- a/Documentation/git-difftool.txt +++ b/Documentation/git-difftool.txt @@ -105,9 +105,6 @@ See linkgit:git-diff[1] for the full list of supported options. CONFIG VARIABLES -'git difftool' falls back to 'git mergetool' config variables when the -difftool equivalents have not been defined. - diff.tool:: The default diff tool to use. -- 2.12.0.5.gfbc750a84
Re: [PATCH v2 2/5] Use -y where possible in test t7610-mergetool
On Fri, Mar 03, 2017 at 11:39:30AM -0800, Junio C Hamano wrote: > Denton Liu <liu.den...@gmail.com> writes: > > > In these tests, there are many situations where > > 'echo "" | git mergetool' is used. This replaces all of those > > occurrences with 'git mergetool -y' for simplicity and readability. > > "-y where _possible_" is not necessarily a good thing to do in > tests. We do want to make sure that both "yes" from the input and > "-y" from the command line work. Changes to "-y" done only for the > tests that are not about accepting the interactive input from the > end-user is a very good idea, but "replaces all of those" makes me > worried. The 'custom mergetool' test case seems like a good place to introduce an interactive test. Would the following patch to my patch work? diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index b6a419258..71624583c 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -135,8 +135,8 @@ test_expect_success 'custom mergetool' ' test_expect_code 1 git merge master >/dev/null 2>&1 && git mergetool -y both >/dev/null 2>&1 && git mergetool -y file1 file1 && - git mergetool -y file2 "spaced name" >/dev/null 2>&1 && - git mergetool -y subdir/file3 >/dev/null 2>&1 && + ( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) && + ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) && ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) && ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) && ( yes "l" | git mergetool submod >/dev/null 2>&1 ) && > > - ( yes "" | git mergetool file1 >/dev/null 2>&1 ) && > > - ( yes "" | git mergetool file2 >/dev/null 2>&1 ) && > > - ( yes "" | git mergetool "spaced name" >/dev/null 2>&1 ) && > > - ( yes "" | git mergetool both >/dev/null 2>&1 ) && > > - ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) && > > + git mergetool file1 >/dev/null 2>&1 && > > + git mergetool file2 >/dev/null 2>&1 && > > + git mergetool "spaced name" >/dev/null 2>&1 && > > + git mergetool both >/dev/null 2>&1 && > > + git mergetool subdir/file3 >/dev/null 2>&1 && > > The reason for the lack of "-y" in the rewrite, in contrast to what > was done in the previous hunk we saw, is not quite obvious. > Sorry, it was my mistake. I had forgotten to replace the '-y'. I have corrected this for a future revision.
Re: [PATCH 3/3] Remove outdated info in difftool manpage
On Fri, Mar 03, 2017 at 04:46:36PM +0100, Johannes Schindelin wrote: > Hi Denton (or should I address you as Liu?), Denton is fine, thanks. > > On Fri, 3 Mar 2017, Denton Liu wrote: > > > When difftool was rewritten in C, it removed the capability to read > > fallback configs from mergetool. This changes the documentation to > > reflect this. > > Thanks for pointing that out. But that is probably an oversight on my > part, not an intentional change... Do you expect to be submitting a patch for this soon? Or, if not, would it be fine if I went ahead and did it? > > Ciao, > Johannes
[PATCH 3/3] Remove outdated info in difftool manpage
When difftool was rewritten in C, it removed the capability to read fallback configs from mergetool. This changes the documentation to reflect this. Signed-off-by: Denton Liu <liu.den...@gmail.com> --- Documentation/git-difftool.txt | 3 --- 1 file changed, 3 deletions(-) diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index 96c26e6aa..a00cb033e 100644 --- a/Documentation/git-difftool.txt +++ b/Documentation/git-difftool.txt @@ -105,9 +105,6 @@ See linkgit:git-diff[1] for the full list of supported options. CONFIG VARIABLES -'git difftool' falls back to 'git mergetool' config variables when the -difftool equivalents have not been defined. - diff.tool:: The default diff tool to use. -- 2.12.0.1.g5415fdfc5.dirty
[PATCH 2/3] Add gui completions for difftool
Signed-off-by: Denton Liu <liu.den...@gmail.com> --- contrib/completion/git-completion.bash | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index d5f3b9aeb..c94e38a3a 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1277,7 +1277,7 @@ _git_difftool () --base --ours --theirs --no-renames --diff-filter= --find-copies-harder --relative --ignore-submodules - --tool=" + --tool= --gui" return ;; esac @@ -2207,6 +2207,7 @@ _git_config () diff.submodule diff.suppressBlankEmpty diff.tool + diff.guitool diff.wordRegex diff.algorithm difftool. -- 2.12.0.1.g5415fdfc5.dirty
[PATCH 1/3] Add --gui option to mergetool
This fixes the discrepancy between difftool and mergetool where the
former has the --gui flag and the latter does not by adding the
functionality to mergetool.
Signed-off-by: Denton Liu <liu.den...@gmail.com>
---
Documentation/git-mergetool.txt| 8 +++-
contrib/completion/git-completion.bash | 3 ++-
git-mergetool.sh | 5 -
t/t7610-mergetool.sh | 28 +++-
4 files changed, 40 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 3622d6648..2ab56efcf 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -8,7 +8,7 @@ git-mergetool - Run merge conflict resolution tools to resolve
merge conflicts
SYNOPSIS
[verse]
-'git mergetool' [--tool=] [-y | --[no-]prompt] [...]
+'git mergetool' [--tool=] [-g|--gui] [-y | --[no-]prompt] [...]
DESCRIPTION
---
@@ -64,6 +64,12 @@ variable `mergetool..trustExitCode` can be set to
`true`.
Otherwise, 'git mergetool' will prompt the user to indicate the
success of the resolution after the custom tool has exited.
+-g::
+--gui::
+ When 'git-mergetool' is invoked with the `-g` or `--gui` option
+ the default diff tool will be read from the configured
+ `merge.guitool` variable instead of `merge.tool`.
+
--tool-help::
Print a list of merge tools that may be used with `--tool`.
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 41ee52991..d5f3b9aeb 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1584,7 +1584,7 @@ _git_mergetool ()
return
;;
--*)
- __gitcomp "--tool= --prompt --no-prompt"
+ __gitcomp "--tool= --prompt --no-prompt --gui"
return
;;
esac
@@ -2308,6 +2308,7 @@ _git_config ()
merge.renormalize
merge.stat
merge.tool
+ merge.guitool
merge.verbosity
mergetool.
mergetool.keepBackup
diff --git a/git-mergetool.sh b/git-mergetool.sh
index c062e3de3..f3fce528b 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -9,7 +9,7 @@
# at the discretion of Junio C Hamano.
#
-USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [-O]
[file to merge] ...'
+USAGE='[--tool=tool] [-g|--gui] [--tool-help] [-y|--no-prompt|--prompt]
[-O] [file to merge] ...'
SUBDIRECTORY_OK=Yes
NONGIT_OK=Yes
OPTIONS_SPEC=
@@ -414,6 +414,9 @@ main () {
shift ;;
esac
;;
+ -g|--gui)
+ merge_tool=$(git config merge.guitool)
+ ;;
-y|--no-prompt)
prompt=false
;;
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 381b7df45..5683907ab 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -123,7 +123,9 @@ test_expect_success 'setup' '
git config mergetool.mytool.cmd "cat \"\$REMOTE\" >\"\$MERGED\"" &&
git config mergetool.mytool.trustExitCode true &&
git config mergetool.mybase.cmd "cat \"\$BASE\" >\"\$MERGED\"" &&
- git config mergetool.mybase.trustExitCode true
+ git config mergetool.mybase.trustExitCode true &&
+ git config mergetool.badtool.cmd false &&
+ git config mergetool.badtool.trustExitCode true
'
test_expect_success 'custom mergetool' '
@@ -145,6 +147,30 @@ test_expect_success 'custom mergetool' '
git commit -m "branch1 resolved with mergetool"
'
+test_expect_success 'gui mergetool' '
+ test_when_finished "git reset --hard" &&
+ test_when_finished "git config merge.tool mytool" &&
+ test_when_finished "git config --unset merge.guitool" &&
+ git config merge.tool badtool &&
+ git config merge.guitool mytool &&
+ git checkout -b test$test_count branch1 &&
+ git submodule update -N &&
+ test_must_fail git merge master >/dev/null 2>&1 &&
+ ( yes "" | git mergetool -g both >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool -g file1 file1 ) &&
+ ( yes "" | git mergetool --gui file2 "spaced name" >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool --gui subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "d" | git mergetool -g file11 >/dev/null 2>&1 ) &&
+ ( yes "d" | git mergetool --gui file12 >/dev/null 2>&a
[RFC PATCH 2/2] completion: add bash completion for 'filter-branch'
Signed-off-by: Denton Liu <liu.den...@gmail.com>
---
If the last patch (PATCH 1/2) is not included, we can remove the call to
__git_complete_rev_list_command.
---
contrib/completion/git-completion.bash | 17 +
1 file changed, 17 insertions(+)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 412485369..933dac78b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1307,6 +1307,23 @@ _git_fetch ()
__git_complete_remote_or_refspec
}
+__git_filter_branch_options="
+ --env-filter --tree-filter --index-filter --parent-filter --msg-filter
+ --commit-filter --tag-name-filter --subdirectory-filter --prune-empty
+ --original --force
+"
+_git_filter_branch ()
+{
+ __git_has_doubledash && __git_complete_rev_list_command && return
+
+ case "$cur" in
+ --*)
+ __gitcomp "$__git_filter_branch_options"
+ return
+ ;;
+ esac
+}
+
__git_format_patch_options="
--stdout --attach --no-attach --thread --thread= --no-thread
--numbered --start-number --numbered-files --keep-subject --signoff
--
2.12.0.1.g5415fdfc5.dirty
[PATCH v2 3/5] Add --gui option to mergetool
This fixes the discrepancy between difftool and mergetool where the
former has the --gui flag and the latter does not by adding the
functionality to mergetool.
Signed-off-by: Denton Liu <liu.den...@gmail.com>
---
Documentation/git-mergetool.txt | 8 +++-
git-mergetool.sh| 5 -
t/t7610-mergetool.sh| 28 +++-
3 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 3622d6648..2ab56efcf 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -8,7 +8,7 @@ git-mergetool - Run merge conflict resolution tools to resolve
merge conflicts
SYNOPSIS
[verse]
-'git mergetool' [--tool=] [-y | --[no-]prompt] [...]
+'git mergetool' [--tool=] [-g|--gui] [-y | --[no-]prompt] [...]
DESCRIPTION
---
@@ -64,6 +64,12 @@ variable `mergetool..trustExitCode` can be set to
`true`.
Otherwise, 'git mergetool' will prompt the user to indicate the
success of the resolution after the custom tool has exited.
+-g::
+--gui::
+ When 'git-mergetool' is invoked with the `-g` or `--gui` option
+ the default diff tool will be read from the configured
+ `merge.guitool` variable instead of `merge.tool`.
+
--tool-help::
Print a list of merge tools that may be used with `--tool`.
diff --git a/git-mergetool.sh b/git-mergetool.sh
index c062e3de3..f3fce528b 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -9,7 +9,7 @@
# at the discretion of Junio C Hamano.
#
-USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [-O]
[file to merge] ...'
+USAGE='[--tool=tool] [-g|--gui] [--tool-help] [-y|--no-prompt|--prompt]
[-O] [file to merge] ...'
SUBDIRECTORY_OK=Yes
NONGIT_OK=Yes
OPTIONS_SPEC=
@@ -414,6 +414,9 @@ main () {
shift ;;
esac
;;
+ -g|--gui)
+ merge_tool=$(git config merge.guitool)
+ ;;
-y|--no-prompt)
prompt=false
;;
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 01c1d44a9..31610f3b0 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -123,7 +123,9 @@ test_expect_success 'setup' '
git config mergetool.mytool.cmd "cat \"\$REMOTE\" >\"\$MERGED\"" &&
git config mergetool.mytool.trustExitCode true &&
git config mergetool.mybase.cmd "cat \"\$BASE\" >\"\$MERGED\"" &&
- git config mergetool.mybase.trustExitCode true
+ git config mergetool.mybase.trustExitCode true &&
+ git config mergetool.badtool.cmd false &&
+ git config mergetool.badtool.trustExitCode true
'
test_expect_success 'custom mergetool' '
@@ -145,6 +147,30 @@ test_expect_success 'custom mergetool' '
git commit -m "branch1 resolved with mergetool"
'
+test_expect_success 'gui mergetool' '
+ test_when_finished "git reset --hard" &&
+ test_when_finished "git config merge.tool mytool" &&
+ test_when_finished "git config --unset merge.guitool" &&
+ git config merge.tool badtool &&
+ git config merge.guitool mytool &&
+ git checkout -b test$test_count branch1 &&
+ git submodule update -N &&
+ test_expect_code 1 git merge master >/dev/null 2>&1 &&
+ git mergetool -g both >/dev/null 2>&1 &&
+ git mergetool -g file1 file1 &&
+ git mergetool --gui file2 "spaced name" >/dev/null 2>&1 &&
+ git mergetool --gui subdir/file3 >/dev/null 2>&1 &&
+ ( yes "d" | git mergetool -g file11 >/dev/null 2>&1 ) &&
+ ( yes "d" | git mergetool --gui file12 >/dev/null 2>&1 ) &&
+ ( yes "l" | git mergetool --gui submod >/dev/null 2>&1 ) &&
+ cat file1 &&
+ test "$(cat file1)" = "master updated" &&
+ test "$(cat file2)" = "master new" &&
+ test "$(cat subdir/file3)" = "master new sub" &&
+ test "$(cat submod/bar)" = "branch1 submodule" &&
+ git commit -m "branch1 resolved with gui mergetool"
+'
+
test_expect_success 'mergetool crlf' '
test_when_finished "git reset --hard" &&
# This test_config line must go after the above reset line so that
--
2.12.0.5.gfbc750a84
[PATCH v2 1/5] Detect merges specifically in test t7610-mergetool
Prior to this, the test cases would use 'test_expect_failure' to detect a merge conflict. This changes the test case to use 'test_expect_code 1' which is more specific. Signed-off-by: Denton Liu <liu.den...@gmail.com> --- t/t7610-mergetool.sh | 76 ++-- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index 381b7df45..a9e274add 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -130,7 +130,7 @@ test_expect_success 'custom mergetool' ' test_when_finished "git reset --hard" && git checkout -b test$test_count branch1 && git submodule update -N && - test_must_fail git merge master >/dev/null 2>&1 && + test_expect_code 1 git merge master >/dev/null 2>&1 && ( yes "" | git mergetool both >/dev/null 2>&1 ) && ( yes "" | git mergetool file1 file1 ) && ( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) && @@ -153,7 +153,7 @@ test_expect_success 'mergetool crlf' ' # test_when_finished is LIFO.) test_config core.autocrlf true && git checkout -b test$test_count branch1 && - test_must_fail git merge master >/dev/null 2>&1 && + test_expect_code 1 git merge master >/dev/null 2>&1 && ( yes "" | git mergetool file1 >/dev/null 2>&1 ) && ( yes "" | git mergetool file2 >/dev/null 2>&1 ) && ( yes "" | git mergetool "spaced name" >/dev/null 2>&1 ) && @@ -176,7 +176,7 @@ test_expect_success 'mergetool in subdir' ' git submodule update -N && ( cd subdir && - test_must_fail git merge master >/dev/null 2>&1 && + test_expect_code 1 git merge master >/dev/null 2>&1 && ( yes "" | git mergetool file3 >/dev/null 2>&1 ) && test "$(cat file3)" = "master new sub" ) @@ -188,7 +188,7 @@ test_expect_success 'mergetool on file in parent dir' ' git submodule update -N && ( cd subdir && - test_must_fail git merge master >/dev/null 2>&1 && + test_expect_code 1 git merge master >/dev/null 2>&1 && ( yes "" | git mergetool file3 >/dev/null 2>&1 ) && ( yes "" | git mergetool ../file1 >/dev/null 2>&1 ) && ( yes "" | git mergetool ../file2 ../spaced\ name >/dev/null 2>&1 ) && @@ -207,7 +207,7 @@ test_expect_success 'mergetool skips autoresolved' ' test_when_finished "git reset --hard" && git checkout -b test$test_count branch1 && git submodule update -N && - test_must_fail git merge master && + test_expect_code 1 git merge master && test -n "$(git ls-files -u)" && ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) && ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) && @@ -222,7 +222,7 @@ test_expect_success 'mergetool merges all from subdir (rerere disabled)' ' test_config rerere.enabled false && ( cd subdir && - test_must_fail git merge master && + test_expect_code 1 git merge master && ( yes "r" | git mergetool ../submod ) && ( yes "d" "d" | git mergetool --no-prompt ) && test "$(cat ../file1)" = "master updated" && @@ -241,7 +241,7 @@ test_expect_success 'mergetool merges all from subdir (rerere enabled)' ' rm -rf .git/rr-cache && ( cd subdir && - test_must_fail git merge master && + test_expect_code 1 git merge master && ( yes "r" | git mergetool ../submod ) && ( yes "d" "d" | git mergetool --no-prompt ) && test "$(cat ../file1)" = "master updated" && @@ -259,7 +259,7 @@ test_expect_success 'mergetool skips resolved paths when rerere is active' ' rm -rf .git/rr-cache && git checkout -b test$test_count branch1 && git submodule update -N && - test_must_fail git merge master >/dev/null 2>&1 &
[PATCH v2 2/5] Use -y where possible in test t7610-mergetool
In these tests, there are many situations where 'echo "" | git mergetool' is used. This replaces all of those occurrences with 'git mergetool -y' for simplicity and readability. Signed-off-by: Denton Liu <liu.den...@gmail.com> --- t/t7610-mergetool.sh | 62 ++-- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index a9e274add..01c1d44a9 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -131,10 +131,10 @@ test_expect_success 'custom mergetool' ' git checkout -b test$test_count branch1 && git submodule update -N && test_expect_code 1 git merge master >/dev/null 2>&1 && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && - ( yes "" | git mergetool file1 file1 ) && - ( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) && - ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) && + git mergetool -y both >/dev/null 2>&1 && + git mergetool -y file1 file1 && + git mergetool -y file2 "spaced name" >/dev/null 2>&1 && + git mergetool -y subdir/file3 >/dev/null 2>&1 && ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) && ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) && ( yes "l" | git mergetool submod >/dev/null 2>&1 ) && @@ -154,11 +154,11 @@ test_expect_success 'mergetool crlf' ' test_config core.autocrlf true && git checkout -b test$test_count branch1 && test_expect_code 1 git merge master >/dev/null 2>&1 && - ( yes "" | git mergetool file1 >/dev/null 2>&1 ) && - ( yes "" | git mergetool file2 >/dev/null 2>&1 ) && - ( yes "" | git mergetool "spaced name" >/dev/null 2>&1 ) && - ( yes "" | git mergetool both >/dev/null 2>&1 ) && - ( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) && + git mergetool file1 >/dev/null 2>&1 && + git mergetool file2 >/dev/null 2>&1 && + git mergetool "spaced name" >/dev/null 2>&1 && + git mergetool both >/dev/null 2>&1 && + git mergetool subdir/file3 >/dev/null 2>&1 && ( yes "d" | git mergetool file11 >/dev/null 2>&1 ) && ( yes "d" | git mergetool file12 >/dev/null 2>&1 ) && ( yes "r" | git mergetool submod >/dev/null 2>&1 ) && @@ -177,7 +177,7 @@ test_expect_success 'mergetool in subdir' ' ( cd subdir && test_expect_code 1 git merge master >/dev/null 2>&1 && - ( yes "" | git mergetool file3 >/dev/null 2>&1 ) && + git mergetool file3 >/dev/null 2>&1 && test "$(cat file3)" = "master new sub" ) ' @@ -189,10 +189,10 @@ test_expect_success 'mergetool on file in parent dir' ' ( cd subdir && test_expect_code 1 git merge master >/dev/null 2>&1 && - ( yes "" | git mergetool file3 >/dev/null 2>&1 ) && - ( yes "" | git mergetool ../file1 >/dev/null 2>&1 ) && - ( yes "" | git mergetool ../file2 ../spaced\ name >/dev/null 2>&1 ) && - ( yes "" | git mergetool ../both >/dev/null 2>&1 ) && + git mergetool file3 >/dev/null 2>&1 && + git mergetool ../file1 >/dev/null 2>&1 && + git mergetool ../file2 ../spaced\ name >/dev/null 2>&1 && + git mergetool ../both >/dev/null 2>&1 && ( yes "d" | git mergetool ../file11 >/dev/null 2>&1 ) && ( yes "d" | git mergetool ../file12 >/dev/null 2>&1 ) && ( yes "l" | git mergetool ../submod >/dev/null 2>&1 ) && @@ -302,7 +302,7 @@ test_expect_success 'mergetool takes partial path' ' git submodule update -N && test_expect_code 1 git merge master && - ( yes "" | git mergetool subdir ) && + git mergetool subdir &&
[PATCH v2 4/5] Add guitool completions for diff and mergetools
Signed-off-by: Denton Liu <liu.den...@gmail.com> --- contrib/completion/git-completion.bash | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 41ee52991..c94e38a3a 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1277,7 +1277,7 @@ _git_difftool () --base --ours --theirs --no-renames --diff-filter= --find-copies-harder --relative --ignore-submodules - --tool=" + --tool= --gui" return ;; esac @@ -1584,7 +1584,7 @@ _git_mergetool () return ;; --*) - __gitcomp "--tool= --prompt --no-prompt" + __gitcomp "--tool= --prompt --no-prompt --gui" return ;; esac @@ -2207,6 +2207,7 @@ _git_config () diff.submodule diff.suppressBlankEmpty diff.tool + diff.guitool diff.wordRegex diff.algorithm difftool. @@ -2308,6 +2309,7 @@ _git_config () merge.renormalize merge.stat merge.tool + merge.guitool merge.verbosity mergetool. mergetool.keepBackup -- 2.12.0.5.gfbc750a84
[PATCH v3 1/3] mergetool: accept -g/--[no-]gui as arguments
In line with how difftool accepts a -g/--[no-]gui option, make mergetool
accept the same option in order to use the `merge.guitool` variable to
find the default mergetool instead of `merge.tool`.
Signed-off-by: Denton Liu
Signed-off-by: Anmol Mago
Signed-off-by: Brian Ho
Signed-off-by: David Lu
Signed-off-by: Ryan Wang
Acked-by: David Aguilar
---
Documentation/git-mergetool.txt | 11 +++
git-mergetool--lib.sh | 16 +++-
git-mergetool.sh| 11 +--
3 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 3622d6648..0c7975a05 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -79,6 +79,17 @@ success of the resolution after the custom tool has exited.
Prompt before each invocation of the merge resolution program
to give the user a chance to skip the path.
+-g::
+--gui::
+ When 'git-mergetool' is invoked with the `-g` or `--gui` option
+ the default merge tool will be read from the configured
+ `merge.guitool` variable instead of `merge.tool`.
+
+--no-gui::
+ This overrides a previous `-g` or `--gui` setting and reads the
+ default merge tool will be read from the configured `merge.tool`
+ variable.
+
-O::
Process files in the order specified in the
, which has one shell glob pattern per line.
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 9a8b97a2a..83bf52494 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -350,17 +350,23 @@ guess_merge_tool () {
}
get_configured_merge_tool () {
- # Diff mode first tries diff.tool and falls back to merge.tool.
- # Merge mode only checks merge.tool
+ # If first argument is true, find the guitool instead
+ if test "$1" = true
+ then
+ gui_prefix=gui
+ fi
+
+ # Diff mode first tries diff.(gui)tool and falls back to
merge.(gui)tool.
+ # Merge mode only checks merge.(gui)tool
if diff_mode
then
- merge_tool=$(git config diff.tool || git config merge.tool)
+ merge_tool=$(git config diff.${gui_prefix}tool || git config
merge.${gui_prefix}tool)
else
- merge_tool=$(git config merge.tool)
+ merge_tool=$(git config merge.${gui_prefix}tool)
fi
if test -n "$merge_tool" && ! valid_tool "$merge_tool"
then
- echo >&2 "git config option $TOOL_MODE.tool set to unknown
tool: $merge_tool"
+ echo >&2 "git config option $TOOL_MODE.${gui_prefix}tool set to
unknown tool: $merge_tool"
echo >&2 "Resetting to default..."
return 1
fi
diff --git a/git-mergetool.sh b/git-mergetool.sh
index d07c7f387..01b9ad59b 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -9,7 +9,7 @@
# at the discretion of Junio C Hamano.
#
-USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [-O]
[file to merge] ...'
+USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt]
[-g|--gui|--no-gui] [-O] [file to merge] ...'
SUBDIRECTORY_OK=Yes
NONGIT_OK=Yes
OPTIONS_SPEC=
@@ -389,6 +389,7 @@ print_noop_and_exit () {
main () {
prompt=$(git config --bool mergetool.prompt)
+ gui_tool=false
guessed_merge_tool=false
orderfile=
@@ -414,6 +415,12 @@ main () {
shift ;;
esac
;;
+ --no-gui)
+ gui_tool=false
+ ;;
+ -g|--gui)
+ gui_tool=true
+ ;;
-y|--no-prompt)
prompt=false
;;
@@ -443,7 +450,7 @@ main () {
if test -z "$merge_tool"
then
# Check if a merge tool has been configured
- merge_tool=$(get_configured_merge_tool)
+ merge_tool=$(get_configured_merge_tool $gui_tool)
# Try to guess an appropriate merge tool if no tool has been
set.
if test -z "$merge_tool"
then
--
2.19.1.544.ge0b0585a1.dirty
[PATCH v3 0/3] Add --gui to mergetool
This adds another patch on top of the existing patchset in order to document the guitool keys for `git config`. This way, the completions script will now be able to complete these key values as well. Denton Liu (3): mergetool: accept -g/--[no-]gui as arguments completion: support `git mergetool --[no-]gui` doc: document diff/merge.guitool config keys Documentation/diff-config.txt | 8 Documentation/git-mergetool.txt| 11 +++ Documentation/merge-config.txt | 6 ++ contrib/completion/git-completion.bash | 2 +- git-mergetool--lib.sh | 16 +++- git-mergetool.sh | 11 +-- 6 files changed, 46 insertions(+), 8 deletions(-) -- 2.19.1.544.ge0b0585a1.dirty
[PATCH v3 2/3] completion: support `git mergetool --[no-]gui`
Signed-off-by: Denton Liu Signed-off-by: Anmol Mago Signed-off-by: Brian Ho Signed-off-by: David Lu Signed-off-by: Ryan Wang --- contrib/completion/git-completion.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index db7fd87b6..a45b4a050 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1833,7 +1833,7 @@ _git_mergetool () return ;; --*) - __gitcomp "--tool= --prompt --no-prompt" + __gitcomp "--tool= --prompt --no-prompt --gui --no-gui" return ;; esac -- 2.19.1.544.ge0b0585a1.dirty
[PATCH v3 3/3] doc: document diff/merge.guitool config keys
Signed-off-by: Denton Liu --- Documentation/diff-config.txt | 8 Documentation/merge-config.txt | 6 ++ 2 files changed, 14 insertions(+) diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt index 85bca83c3..e64d983c3 100644 --- a/Documentation/diff-config.txt +++ b/Documentation/diff-config.txt @@ -177,6 +177,14 @@ diff.tool:: Any other value is treated as a custom diff tool and requires that a corresponding difftool..cmd variable is defined. +diff.guitool:: + Controls which diff tool is used by linkgit:git-difftool[1] when + the -g/--gui flag is specified. This variable overrides the value + configured in `merge.guitool`. The list below shows the valid + built-in values. Any other value is treated as a custom diff tool + and requires that a corresponding difftool..cmd variable + is defined. + include::mergetools-diff.txt[] diff.indentHeuristic:: diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt index 662c2713c..a7f4ea90c 100644 --- a/Documentation/merge-config.txt +++ b/Documentation/merge-config.txt @@ -63,6 +63,12 @@ merge.tool:: Any other value is treated as a custom merge tool and requires that a corresponding mergetool..cmd variable is defined. +merge.guitool:: + Controls which merge tool is used by linkgit:git-mergetool[1] when the + -g/--gui flag is specified. The list below shows the valid built-in values. + Any other value is treated as a custom merge tool and requires that a + corresponding mergetool..cmd variable is defined. + include::mergetools-merge.txt[] merge.verbosity:: -- 2.19.1.544.ge0b0585a1.dirty
Re: [PATCH] completion: use builtin completion for format-patch
On Tue, Oct 30, 2018 at 11:20:45AM +0900, Junio C Hamano wrote: > We saw a similar change proposed and then found out it was not such > a good idea in: > > https://public-inbox.org/git/cacsjy8durvju0hn7kuceo4iv5aimwbytr+e-7kenpvdx90d...@mail.gmail.com/ > > It seems that this one loses options like --full-index, --no-prefix, > etc. compared to the earlier effort? In _git_send_email, we have the following lines: __gitcomp "--annotate --bcc --cc --cc-cmd --chain-reply-to ... more options ... $__git_format_patch_options" Would it make sense to take the old `__git_format_patch_options` and just roll them into here, then make `_git_format_patch` use `__gitcomp_builtin format-patch`? That way, we'd be able to reap the benefits of using `__gitcomp_builtin` where we can.
[PATCH v2] completion: use builtin completion for format-patch
This patch offloads completion functionality for format-patch to
__gitcomp_builtin. In addition to this, send-email was borrowing some
completion options from format-patch so those options are moved into
send-email's completions.
Signed-off-by: Denton Liu
---
I ran t9902-completion.sh on this patch and it seems to pass. Thus, this
should address the concerns about losing some completion options in
send-email.
---
contrib/completion/git-completion.bash | 34 +++---
1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index d63d2dffd..cb4ef6723 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1531,15 +1531,6 @@ _git_fetch ()
__git_complete_remote_or_refspec
}
-__git_format_patch_options="
- --stdout --attach --no-attach --thread --thread= --no-thread
- --numbered --start-number --numbered-files --keep-subject --signoff
- --signature --no-signature --in-reply-to= --cc= --full-index --binary
- --not --all --cover-letter --no-prefix --src-prefix= --dst-prefix=
- --inline --suffix= --ignore-if-in-upstream --subject-prefix=
- --output-directory --reroll-count --to= --quiet --notes
-"
-
_git_format_patch ()
{
case "$cur" in
@@ -1550,7 +1541,7 @@ _git_format_patch ()
return
;;
--*)
- __gitcomp "$__git_format_patch_options"
+ __gitcomp_builtin format-patch
return
;;
esac
@@ -2080,16 +2071,19 @@ _git_send_email ()
return
;;
--*)
- __gitcomp "--annotate --bcc --cc --cc-cmd --chain-reply-to
- --compose --confirm= --dry-run --envelope-sender
- --from --identity
- --in-reply-to --no-chain-reply-to --no-signed-off-by-cc
- --no-suppress-from --no-thread --quiet --reply-to
- --signed-off-by-cc --smtp-pass --smtp-server
- --smtp-server-port --smtp-encryption= --smtp-user
- --subject --suppress-cc= --suppress-from --thread --to
- --validate --no-validate
- $__git_format_patch_options"
+ __gitcomp "--all --annotate --attach --bcc --binary --cc --cc=
--cc-cmd
+ --chain-reply-to --compose --confirm= --cover-letter
--dry-run
+ --dst-prefix= --envelope-sender --from --full-index
--identity
+ --ignore-if-in-upstream --inline --in-reply-to
--in-reply-to=
+ --keep-subject --no-attach --no-chain-reply-to
--no-prefix
+ --no-signature --no-signed-off-by-cc --no-suppress-from
--not --notes
+ --no-thread --no-validate --numbered --numbered-files
+ --output-directory --quiet --reply-to --reroll-count
--signature
+ --signed-off-by-cc --signoff --smtp-encryption=
--smtp-pass
+ --smtp-server --smtp-server-port --smtp-user
--src-prefix=
+ --start-number --stdout --subject --subject-prefix=
--suffix=
+ --suppress-cc= --suppress-from --thread --thread= --to
--to=
+ --validate"
return
;;
esac
--
2.19.1
Re: [RFC PATCH] remote: add --fetch option to git remote set-url
On Mon, Oct 29, 2018 at 02:57:28PM +0900, Junio C Hamano wrote:
> Denton Liu writes:
>
> > This adds the --fetch option to `git remote set-url` such that when
> > executed we move the remote.*.url to remote.*.pushurl and set
> > remote.*.url to the given url argument.
> >
>
> I suspect this is a horrible idea from end-user's point of view.
> "set-url --push" is used to SET pushURL instead of setting URL and
> does not MOVE anything. Why should the end user expect and remember
> "set-url --fetch" works very differently?
>
I agree, `--fetch` is a terrible name for this. Perhaps a better name
would be something like `--fetch-behavior` or `--keep-push` so that the
behaviour is more transparent for the end-user. Either way, I think we
can decide on the name later.
> If there is a need for a "--move-URL-to-pushURL-and-set-pushURL"
> short-hand to avoid having to use two commands
>
> git remote set-url --push $(git remote --get-url origin) origin
> git remote set-url $there origin
>
> it should not be called "--fetch", which has a strong connotation of
> being an opposite of existing "--push", but something else. And
> then we need to ask ourselves if we also need such a short-hand to
> "--move-pushURL-to-URL-and-set-URL" operation. The answer to the
> last question would help us decide if (1) this combined operation is
> a good idea to begin with and (2) what is the good name for such an
> operation.
>
> Assuming that the short-hand operation is a good idea in the first
> place, without deciding what the externally visible good name for it
> is, let's read on.
>
> > + /*
> > +* If add_mode, we will be appending to remote.*.url so we shouldn't
> > move the urls over.
> > +* If pushurls exist, we don't need to move the urls over to pushurl.
> > +*/
> > + move_fetch_to_push = fetch_mode && !add_mode && !remote->pushurl_nr;
>
> Should this kind of "the user asked for --fetch, but sometimes it is
> not appropriate to honor that request" be done silently like this?
>
> Earlier you had a check like this:
>
> > + if (push_mode && fetch_mode)
> > + die(_("--push --fetch doesn't make sense"));
>
> If a request to "--fetch" is ignored when "--add" is given, for
> example, shouldn't the combination also be diagnosed as "not making
> sense, we'd ignore your wish to use the --fetch option"? Similarly
> for the case where there already is pushurl defined for the remote.
>
> This is a different tangent on the same line, but it could be that
> the user wants to have two (or more) pushURLs because the user wants
> to push to two remotes at the same time with "git push this-remote",
> so silently ignoring "--force" may not be the right thing in the
> first place. We may instead need to make the value of URL to an
> extra pushURL entry (if we had one, we now have two).
>
Perhaps I should motivate the use-case for this option. There have been
times when I've had the URL set to something but no pushURL. I've
wanted to only change the fetching URL and keep the pushing URL the same
but unfortunately, there's no way to do that because of the url/pushurl
split is set up.
My implementation of --fetch is supposed to emulate what would happen if
git were implemented with fetchurl/pushurl instead. Does the patch make
more sense in this context?
Please let me know if you think that the concept behind this patch is a
good idea. Thanks!
[RFC PATCH] remote: add --fetch option to git remote set-url
This adds the --fetch option to `git remote set-url` such that when
executed we move the remote.*.url to remote.*.pushurl and set
remote.*.url to the given url argument.
For example, if we have the following config:
[remote "origin"]
url = g...@github.com:git/git.git
`git remote set-url --fetch origin https://github.com/git/git.git`
would change the config to the following:
[remote "origin"]
url = https://github.com/git/git.git
pushurl = g...@github.com:git/git.git
Signed-off-by: Denton Liu
Signed-off-by: Filip Francetic
---
builtin/remote.c | 42 ++
1 file changed, 34 insertions(+), 8 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index f7edf7f2c..fcf1220c6 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -23,9 +23,9 @@ static const char * const builtin_remote_usage[] = {
N_("git remote [-v | --verbose] update [-p | --prune] [( |
)...]"),
N_("git remote set-branches [--add] ..."),
N_("git remote get-url [--push] [--all] "),
- N_("git remote set-url [--push] []"),
- N_("git remote set-url --add "),
- N_("git remote set-url --delete "),
+ N_("git remote set-url [--push|--fetch] []"),
+ N_("git remote set-url --add [--push|--fetch] "),
+ N_("git remote set-url --delete [--push|--fetch] "),
NULL
};
@@ -76,9 +76,9 @@ static const char * const builtin_remote_geturl_usage[] = {
};
static const char * const builtin_remote_seturl_usage[] = {
- N_("git remote set-url [--push] []"),
- N_("git remote set-url --add "),
- N_("git remote set-url --delete "),
+ N_("git remote set-url [--push|--fetch] []"),
+ N_("git remote set-url --add [--push|--fetch] "),
+ N_("git remote set-url --delete [--push|--fetch] "),
NULL
};
@@ -1519,7 +1519,7 @@ static int get_url(int argc, const char **argv)
static int set_url(int argc, const char **argv)
{
- int i, push_mode = 0, add_mode = 0, delete_mode = 0;
+ int i, push_mode = 0, fetch_mode = 0, add_mode = 0, delete_mode = 0,
move_fetch_to_push = 0;
int matches = 0, negative_matches = 0;
const char *remotename = NULL;
const char *newurl = NULL;
@@ -1532,6 +1532,8 @@ static int set_url(int argc, const char **argv)
struct option options[] = {
OPT_BOOL('\0', "push", _mode,
N_("manipulate push URLs")),
+ OPT_BOOL('\0', "fetch", _mode,
+N_("manipulate fetch URLs")),
OPT_BOOL('\0', "add", _mode,
N_("add URL")),
OPT_BOOL('\0', "delete", _mode,
@@ -1543,6 +1545,8 @@ static int set_url(int argc, const char **argv)
if (add_mode && delete_mode)
die(_("--add --delete doesn't make sense"));
+ if (push_mode && fetch_mode)
+ die(_("--push --fetch doesn't make sense"));
if (argc < 3 || argc > 4 || ((add_mode || delete_mode) && argc != 3))
usage_with_options(builtin_remote_seturl_usage, options);
@@ -1559,18 +1563,40 @@ static int set_url(int argc, const char **argv)
if (!remote_is_configured(remote, 1))
die(_("No such remote '%s'"), remotename);
+ /*
+* If add_mode, we will be appending to remote.*.url so we shouldn't
move the urls over.
+* If pushurls exist, we don't need to move the urls over to pushurl.
+*/
+ move_fetch_to_push = fetch_mode && !add_mode && !remote->pushurl_nr;
+
if (push_mode) {
strbuf_addf(_buf, "remote.%s.pushurl", remotename);
urlset = remote->pushurl;
urlset_nr = remote->pushurl_nr;
} else {
+ if (move_fetch_to_push) {
+ strbuf_addf(_buf, "remote.%s.pushurl", remotename);
+ for (i = 0; i < remote->url_nr; i++) {
+ git_config_set_multivar(name_buf.buf,
remote->url[i],
+ "^$", 0);
+ }
+ strbuf_reset(_buf);
+ }
+
strbuf_addf(_buf, "remote.%s.url", remotename);
urlset = remote->url;
urlset_nr = remote->url_nr;
}
+ /* Empty fetch URLs if they are being replaced */
+ if (move_fetch_to_push) {
+ for (i = 0; i < remote->url_nr; i++) {
+
Re: [PATCH v2] completion: use builtin completion for format-patch
On Sat, Nov 03, 2018 at 07:03:18AM +0100, Duy Nguyen wrote:
> Subject: [PATCH] completion: use __gitcomp_builtin for format-patch
>
> This helps format-patch gain completion for a couple new options,
> notably --range-diff.
>
> Since send-email completion relies on $__git_format_patch_options
> which is now reduced, we need to do something not to regress
> send-email completion.
>
> The workaround here is implement --git-completion-helper in
> send-email.perl just as a bridge to "format-patch --git-completion-helper".
> This is enough to use __gitcomp_builtin on send-email (to take
> advantage of caching).
>
> In the end, send-email.perl can probably reuse the same info it passes
> to GetOptions() to generate full --git-completion-helper output so
> that we don't need to keep track of its options in git-completion.bash
> anymore. But that's something for another boring day.
>
> Helped-by: Denton Liu
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
> contrib/completion/git-completion.bash | 16 ++--
> git-send-email.perl| 8
> 2 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash
> b/contrib/completion/git-completion.bash
> index db7fd87b6b..8409978793 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1532,13 +1532,9 @@ _git_fetch ()
> __git_complete_remote_or_refspec
> }
>
> -__git_format_patch_options="
> - --stdout --attach --no-attach --thread --thread= --no-thread
> - --numbered --start-number --numbered-files --keep-subject --signoff
> - --signature --no-signature --in-reply-to= --cc= --full-index --binary
> - --not --all --cover-letter --no-prefix --src-prefix= --dst-prefix=
> - --inline --suffix= --ignore-if-in-upstream --subject-prefix=
> - --output-directory --reroll-count --to= --quiet --notes
> +__git_format_patch_extra_options="
> + --full-index --not --all --no-prefix --src-prefix=
> + --dst-prefix= --notes
> "
>
> _git_format_patch ()
> @@ -1551,7 +1547,7 @@ _git_format_patch ()
> return
> ;;
> --*)
> - __gitcomp "$__git_format_patch_options"
> + __gitcomp_builtin format-patch
> "$__git_format_patch_extra_options"
> return
> ;;
> esac
> @@ -2081,7 +2077,7 @@ _git_send_email ()
> return
> ;;
> --*)
> - __gitcomp "--annotate --bcc --cc --cc-cmd --chain-reply-to
> + __gitcomp_builtin send-email "--annotate --bcc --cc --cc-cmd
> --chain-reply-to
> --compose --confirm= --dry-run --envelope-sender
> --from --identity
> --in-reply-to --no-chain-reply-to --no-signed-off-by-cc
Would it make sense to make send-email's completion helper print these
out directly? That way, if someone were to modify send-email in the
future, they'd only have to look through one file instead of both
send-email and the completions script.
> @@ -2090,7 +2086,7 @@ _git_send_email ()
> --smtp-server-port --smtp-encryption= --smtp-user
> --subject --suppress-cc= --suppress-from --thread --to
> --validate --no-validate
> - $__git_format_patch_options"
> + $__git_format_patch_extra_options"
> return
> ;;
> esac
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 2be5dac337..ed0714eaaa 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -119,6 +119,11 @@ sub usage {
> exit(1);
> }
>
> +sub completion_helper {
> +print Git::command('format-patch', '--git-completion-helper');
> +exit(0);
> +}
> +
> # most mail servers generate the Date: header, but not all...
> sub format_2822_time {
> my ($time) = @_;
> @@ -311,6 +316,7 @@ sub signal_handler {
> # needing, first, from the command line:
>
> my $help;
> +my $git_completion_helper;
> my $rc = GetOptions("h" => \$help,
> "dump-aliases" => \$dump_aliases);
> usage() unless $rc;
> @@ -373,9 +379,11 @@ sub signal_handler {
> "no-xmailer" => sub {$use_xmailer = 0},
> "batch-size=i" => \$batch_size,
> "relogin-delay=i" => \$relogin_delay,
> + "git-completion-helper" => \$git_completion_helper,
>);
>
> usage() if $help;
> +completion_helper() if $git_completion_helper;
> unless ($rc) {
> usage();
> }
> --
> 2.19.1.1005.gac84295441
>
> -- 8< --
> --
> Duy
Aside from that one comment, it looks good to me. Thanks for helping me
clean up my earlier patch!
[PATCH 1/2] mergetool: Accept -g/--[no-]gui as arguments
In line with how difftool accepts a -g/--[no-]gui option, make mergetool
accept the same option in order to use the `merge.guitool` variable to
find the default mergetool instead of `merge.tool`.
Signed-off-by: Denton Liu
Signed-off-by: Anmol Mago
Signed-off-by: Brian Ho
Signed-off-by: David Lu
Signed-off-by: Ryan Wang
---
Documentation/git-mergetool.txt | 11 +++
git-mergetool--lib.sh | 16 +++-
git-mergetool.sh| 11 +--
3 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 3622d6648..0c7975a05 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -79,6 +79,17 @@ success of the resolution after the custom tool has exited.
Prompt before each invocation of the merge resolution program
to give the user a chance to skip the path.
+-g::
+--gui::
+ When 'git-mergetool' is invoked with the `-g` or `--gui` option
+ the default merge tool will be read from the configured
+ `merge.guitool` variable instead of `merge.tool`.
+
+--no-gui::
+ This overrides a previous `-g` or `--gui` setting and reads the
+ default merge tool will be read from the configured `merge.tool`
+ variable.
+
-O::
Process files in the order specified in the
, which has one shell glob pattern per line.
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 9a8b97a2a..e317ae003 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -350,17 +350,23 @@ guess_merge_tool () {
}
get_configured_merge_tool () {
- # Diff mode first tries diff.tool and falls back to merge.tool.
- # Merge mode only checks merge.tool
+ # If first argument is true, find the guitool instead
+ if [ "$1" = true ]
+ then
+ gui_prefix=gui
+ fi
+
+ # Diff mode first tries diff.(gui)tool and falls back to
merge.(gui)tool.
+ # Merge mode only checks merge.(gui)tool
if diff_mode
then
- merge_tool=$(git config diff.tool || git config merge.tool)
+ merge_tool=$(git config diff.${gui_prefix}tool || git config
merge.${gui_prefix}tool)
else
- merge_tool=$(git config merge.tool)
+ merge_tool=$(git config merge.${gui_prefix}tool)
fi
if test -n "$merge_tool" && ! valid_tool "$merge_tool"
then
- echo >&2 "git config option $TOOL_MODE.tool set to unknown
tool: $merge_tool"
+ echo >&2 "git config option $TOOL_MODE.${gui_prefix}tool set to
unknown tool: $merge_tool"
echo >&2 "Resetting to default..."
return 1
fi
diff --git a/git-mergetool.sh b/git-mergetool.sh
index d07c7f387..01b9ad59b 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -9,7 +9,7 @@
# at the discretion of Junio C Hamano.
#
-USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [-O]
[file to merge] ...'
+USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt]
[-g|--gui|--no-gui] [-O] [file to merge] ...'
SUBDIRECTORY_OK=Yes
NONGIT_OK=Yes
OPTIONS_SPEC=
@@ -389,6 +389,7 @@ print_noop_and_exit () {
main () {
prompt=$(git config --bool mergetool.prompt)
+ gui_tool=false
guessed_merge_tool=false
orderfile=
@@ -414,6 +415,12 @@ main () {
shift ;;
esac
;;
+ --no-gui)
+ gui_tool=false
+ ;;
+ -g|--gui)
+ gui_tool=true
+ ;;
-y|--no-prompt)
prompt=false
;;
@@ -443,7 +450,7 @@ main () {
if test -z "$merge_tool"
then
# Check if a merge tool has been configured
- merge_tool=$(get_configured_merge_tool)
+ merge_tool=$(get_configured_merge_tool $gui_tool)
# Try to guess an appropriate merge tool if no tool has been
set.
if test -z "$merge_tool"
then
--
2.19.1
[PATCH 2/2] completion: Support `git mergetool --[no-]gui`
Signed-off-by: Denton Liu Signed-off-by: Anmol Mago Signed-off-by: Brian Ho Signed-off-by: David Lu Signed-off-by: Ryan Wang --- contrib/completion/git-completion.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index db7fd87b6..a45b4a050 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1833,7 +1833,7 @@ _git_mergetool () return ;; --*) - __gitcomp "--tool= --prompt --no-prompt" + __gitcomp "--tool= --prompt --no-prompt --gui --no-gui" return ;; esac -- 2.19.1
[PATCH v2 1/2] mergetool: accept -g/--[no-]gui as arguments
In line with how difftool accepts a -g/--[no-]gui option, make mergetool
accept the same option in order to use the `merge.guitool` variable to
find the default mergetool instead of `merge.tool`.
Signed-off-by: Denton Liu
Signed-off-by: Anmol Mago
Signed-off-by: Brian Ho
Signed-off-by: David Lu
Signed-off-by: Ryan Wang
Acked-by: David Aguilar
---
Documentation/git-mergetool.txt | 11 +++
git-mergetool--lib.sh | 16 +++-
git-mergetool.sh| 11 +--
3 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 3622d6648..0c7975a05 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -79,6 +79,17 @@ success of the resolution after the custom tool has exited.
Prompt before each invocation of the merge resolution program
to give the user a chance to skip the path.
+-g::
+--gui::
+ When 'git-mergetool' is invoked with the `-g` or `--gui` option
+ the default merge tool will be read from the configured
+ `merge.guitool` variable instead of `merge.tool`.
+
+--no-gui::
+ This overrides a previous `-g` or `--gui` setting and reads the
+ default merge tool will be read from the configured `merge.tool`
+ variable.
+
-O::
Process files in the order specified in the
, which has one shell glob pattern per line.
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 9a8b97a2a..83bf52494 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -350,17 +350,23 @@ guess_merge_tool () {
}
get_configured_merge_tool () {
- # Diff mode first tries diff.tool and falls back to merge.tool.
- # Merge mode only checks merge.tool
+ # If first argument is true, find the guitool instead
+ if test "$1" = true
+ then
+ gui_prefix=gui
+ fi
+
+ # Diff mode first tries diff.(gui)tool and falls back to
merge.(gui)tool.
+ # Merge mode only checks merge.(gui)tool
if diff_mode
then
- merge_tool=$(git config diff.tool || git config merge.tool)
+ merge_tool=$(git config diff.${gui_prefix}tool || git config
merge.${gui_prefix}tool)
else
- merge_tool=$(git config merge.tool)
+ merge_tool=$(git config merge.${gui_prefix}tool)
fi
if test -n "$merge_tool" && ! valid_tool "$merge_tool"
then
- echo >&2 "git config option $TOOL_MODE.tool set to unknown
tool: $merge_tool"
+ echo >&2 "git config option $TOOL_MODE.${gui_prefix}tool set to
unknown tool: $merge_tool"
echo >&2 "Resetting to default..."
return 1
fi
diff --git a/git-mergetool.sh b/git-mergetool.sh
index d07c7f387..01b9ad59b 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -9,7 +9,7 @@
# at the discretion of Junio C Hamano.
#
-USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [-O]
[file to merge] ...'
+USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt]
[-g|--gui|--no-gui] [-O] [file to merge] ...'
SUBDIRECTORY_OK=Yes
NONGIT_OK=Yes
OPTIONS_SPEC=
@@ -389,6 +389,7 @@ print_noop_and_exit () {
main () {
prompt=$(git config --bool mergetool.prompt)
+ gui_tool=false
guessed_merge_tool=false
orderfile=
@@ -414,6 +415,12 @@ main () {
shift ;;
esac
;;
+ --no-gui)
+ gui_tool=false
+ ;;
+ -g|--gui)
+ gui_tool=true
+ ;;
-y|--no-prompt)
prompt=false
;;
@@ -443,7 +450,7 @@ main () {
if test -z "$merge_tool"
then
# Check if a merge tool has been configured
- merge_tool=$(get_configured_merge_tool)
+ merge_tool=$(get_configured_merge_tool $gui_tool)
# Try to guess an appropriate merge tool if no tool has been
set.
if test -z "$merge_tool"
then
--
2.19.1
[PATCH v2 2/2] completion: support `git mergetool --[no-]gui`
Signed-off-by: Denton Liu Signed-off-by: Anmol Mago Signed-off-by: Brian Ho Signed-off-by: David Lu Signed-off-by: Ryan Wang --- contrib/completion/git-completion.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index db7fd87b6..a45b4a050 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1833,7 +1833,7 @@ _git_mergetool () return ;; --*) - __gitcomp "--tool= --prompt --no-prompt" + __gitcomp "--tool= --prompt --no-prompt --gui --no-gui" return ;; esac -- 2.19.1
Re: [PATCH 1/2] mergetool: Accept -g/--[no-]gui as arguments
On Wed, Oct 24, 2018 at 02:10:14PM +0900, Junio C Hamano wrote:
> In mode_ok shell function, we seem to be prepared to deal with a
> case where neither diff_mode nor merge_mode is true. Should this
> codepath also be prepared to?
>
According to Documentation/git-mergetool--lib.txt,
>Before sourcing 'git-mergetool{litdd}lib', your script must set `TOOL_MODE`
>to define the operation mode for the functions listed below.
>'diff' and 'merge' are valid values.
so I think that we can assume that the one of diff_mode or merge_mode
will return true. In any case, it seems like the rest of the code was
written under this assumption so if this needs to be changed then the
whole library needs to be fixed as well.
[RESEND PATCH v3 0/3] Add --gui to mergetool
This is a resend of patchset v3 where we add another patch on top of the existing patchset in order to document the guitool keys for `git config`. This way, the completions script will now be able to complete these key values as well. Denton Liu (3): mergetool: accept -g/--[no-]gui as arguments completion: support `git mergetool --[no-]gui` doc: document diff/merge.guitool config keys Documentation/diff-config.txt | 8 Documentation/git-mergetool.txt| 11 +++ Documentation/merge-config.txt | 6 ++ contrib/completion/git-completion.bash | 2 +- git-mergetool--lib.sh | 16 +++- git-mergetool.sh | 11 +-- 6 files changed, 46 insertions(+), 8 deletions(-) -- 2.19.1
[RESEND PATCH v3 3/3] doc: document diff/merge.guitool config keys
Signed-off-by: Denton Liu --- Documentation/diff-config.txt | 8 Documentation/merge-config.txt | 6 ++ 2 files changed, 14 insertions(+) diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt index 85bca83c3..e64d983c3 100644 --- a/Documentation/diff-config.txt +++ b/Documentation/diff-config.txt @@ -177,6 +177,14 @@ diff.tool:: Any other value is treated as a custom diff tool and requires that a corresponding difftool..cmd variable is defined. +diff.guitool:: + Controls which diff tool is used by linkgit:git-difftool[1] when + the -g/--gui flag is specified. This variable overrides the value + configured in `merge.guitool`. The list below shows the valid + built-in values. Any other value is treated as a custom diff tool + and requires that a corresponding difftool..cmd variable + is defined. + include::mergetools-diff.txt[] diff.indentHeuristic:: diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt index 662c2713c..a7f4ea90c 100644 --- a/Documentation/merge-config.txt +++ b/Documentation/merge-config.txt @@ -63,6 +63,12 @@ merge.tool:: Any other value is treated as a custom merge tool and requires that a corresponding mergetool..cmd variable is defined. +merge.guitool:: + Controls which merge tool is used by linkgit:git-mergetool[1] when the + -g/--gui flag is specified. The list below shows the valid built-in values. + Any other value is treated as a custom merge tool and requires that a + corresponding mergetool..cmd variable is defined. + include::mergetools-merge.txt[] merge.verbosity:: -- 2.19.1
[RESEND PATCH v3 1/3] mergetool: accept -g/--[no-]gui as arguments
In line with how difftool accepts a -g/--[no-]gui option, make mergetool
accept the same option in order to use the `merge.guitool` variable to
find the default mergetool instead of `merge.tool`.
Signed-off-by: Denton Liu
Signed-off-by: Anmol Mago
Signed-off-by: Brian Ho
Signed-off-by: David Lu
Signed-off-by: Ryan Wang
Acked-by: David Aguilar
---
Documentation/git-mergetool.txt | 11 +++
git-mergetool--lib.sh | 16 +++-
git-mergetool.sh| 11 +--
3 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 3622d6648..0c7975a05 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -79,6 +79,17 @@ success of the resolution after the custom tool has exited.
Prompt before each invocation of the merge resolution program
to give the user a chance to skip the path.
+-g::
+--gui::
+ When 'git-mergetool' is invoked with the `-g` or `--gui` option
+ the default merge tool will be read from the configured
+ `merge.guitool` variable instead of `merge.tool`.
+
+--no-gui::
+ This overrides a previous `-g` or `--gui` setting and reads the
+ default merge tool will be read from the configured `merge.tool`
+ variable.
+
-O::
Process files in the order specified in the
, which has one shell glob pattern per line.
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 9a8b97a2a..83bf52494 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -350,17 +350,23 @@ guess_merge_tool () {
}
get_configured_merge_tool () {
- # Diff mode first tries diff.tool and falls back to merge.tool.
- # Merge mode only checks merge.tool
+ # If first argument is true, find the guitool instead
+ if test "$1" = true
+ then
+ gui_prefix=gui
+ fi
+
+ # Diff mode first tries diff.(gui)tool and falls back to
merge.(gui)tool.
+ # Merge mode only checks merge.(gui)tool
if diff_mode
then
- merge_tool=$(git config diff.tool || git config merge.tool)
+ merge_tool=$(git config diff.${gui_prefix}tool || git config
merge.${gui_prefix}tool)
else
- merge_tool=$(git config merge.tool)
+ merge_tool=$(git config merge.${gui_prefix}tool)
fi
if test -n "$merge_tool" && ! valid_tool "$merge_tool"
then
- echo >&2 "git config option $TOOL_MODE.tool set to unknown
tool: $merge_tool"
+ echo >&2 "git config option $TOOL_MODE.${gui_prefix}tool set to
unknown tool: $merge_tool"
echo >&2 "Resetting to default..."
return 1
fi
diff --git a/git-mergetool.sh b/git-mergetool.sh
index d07c7f387..01b9ad59b 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -9,7 +9,7 @@
# at the discretion of Junio C Hamano.
#
-USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [-O]
[file to merge] ...'
+USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt]
[-g|--gui|--no-gui] [-O] [file to merge] ...'
SUBDIRECTORY_OK=Yes
NONGIT_OK=Yes
OPTIONS_SPEC=
@@ -389,6 +389,7 @@ print_noop_and_exit () {
main () {
prompt=$(git config --bool mergetool.prompt)
+ gui_tool=false
guessed_merge_tool=false
orderfile=
@@ -414,6 +415,12 @@ main () {
shift ;;
esac
;;
+ --no-gui)
+ gui_tool=false
+ ;;
+ -g|--gui)
+ gui_tool=true
+ ;;
-y|--no-prompt)
prompt=false
;;
@@ -443,7 +450,7 @@ main () {
if test -z "$merge_tool"
then
# Check if a merge tool has been configured
- merge_tool=$(get_configured_merge_tool)
+ merge_tool=$(get_configured_merge_tool $gui_tool)
# Try to guess an appropriate merge tool if no tool has been
set.
if test -z "$merge_tool"
then
--
2.19.1
[RESEND PATCH v3 2/3] completion: support `git mergetool --[no-]gui`
Signed-off-by: Denton Liu Signed-off-by: Anmol Mago Signed-off-by: Brian Ho Signed-off-by: David Lu Signed-off-by: Ryan Wang --- contrib/completion/git-completion.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index db7fd87b6..a45b4a050 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1833,7 +1833,7 @@ _git_mergetool () return ;; --*) - __gitcomp "--tool= --prompt --no-prompt" + __gitcomp "--tool= --prompt --no-prompt --gui --no-gui" return ;; esac -- 2.19.1
[PATCH] completion: use builtin completion for format-patch
Signed-off-by: Denton Liu
---
contrib/completion/git-completion.bash | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index d63d2dffd..da77da481 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1531,15 +1531,6 @@ _git_fetch ()
__git_complete_remote_or_refspec
}
-__git_format_patch_options="
- --stdout --attach --no-attach --thread --thread= --no-thread
- --numbered --start-number --numbered-files --keep-subject --signoff
- --signature --no-signature --in-reply-to= --cc= --full-index --binary
- --not --all --cover-letter --no-prefix --src-prefix= --dst-prefix=
- --inline --suffix= --ignore-if-in-upstream --subject-prefix=
- --output-directory --reroll-count --to= --quiet --notes
-"
-
_git_format_patch ()
{
case "$cur" in
@@ -1550,7 +1541,7 @@ _git_format_patch ()
return
;;
--*)
- __gitcomp "$__git_format_patch_options"
+ __gitcomp_builtin format-patch
return
;;
esac
--
2.19.1
[PATCH v3] remote: add --save-to-push option to git remote set-url
This adds the --save-to-push option to `git remote set-url` such that
when executed, we move the remote.*.url to remote.*.pushurl and set
remote.*.url to the given url argument.
For example, if we have the following config:
[remote "origin"]
url = g...@github.com:git/git.git
`git remote set-url --save-to-push origin https://github.com/git/git.git`
would change the config to the following:
[remote "origin"]
url = https://github.com/git/git.git
pushurl = g...@github.com:git/git.git
Signed-off-by: Denton Liu
---
On Fri, Nov 09, 2018 at 12:15:22PM +0900, Junio C Hamano wrote:
> This sounds more like "saving to push" (i.e. what you are saving is
> the existing "url" and the "push" is a shorthand for "pushURL",
> which is the location the old value of "url" is aved to), not "save
> (the) push(URL)". So if adding this option makes sense, I would say
> "--save-to-push" (or even "--save-to-pushURL") may be a more
> appropriate name for it.
>
My original intention was for it to mean "save push behavior" but I
agree with you that it's unclear so I'm changing it to --save-to-push.
> Ambigous in what way? You asked the current URL to be saved as a
> pushURL, so existing pushURL destinations should not come into play,
> I would think. If there are more than one URL (not pushURL), on the
> other hand, I think you have a bigger problem (where would "git fetch"
> fetch from, and how would these multiple URLs are prevented from
> trashing refs/remotes/$remote/* with each other's refs?), so
> stopping the operation before "set-url" makes the problem worse is
> probably a good idea, but I think that is true with or without this
> new option.
>
> I _think_ in the future (if this option turns out to be widely used)
> people may ask for this condition to be loosened somewhat, but it is
> relatively easy to start restrictive and then to loosen later, so I
> think this is OK for now.
>
I agree, there's no reason why we shouldn't allow appending to the push
URLs if one already exists so I removed that removed that restriction.
---
Documentation/git-remote.txt | 5 +
builtin/remote.c | 26 +-
t/t5505-remote.sh| 11 +++
3 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index 0cad37fb81..8f9d700252 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -19,6 +19,7 @@ SYNOPSIS
'git remote set-url' [--push] []
'git remote set-url --add' [--push]
'git remote set-url --delete' [--push]
+'git remote set-url --save-to-push'
'git remote' [-v | --verbose] 'show' [-n] ...
'git remote prune' [-n | --dry-run] ...
'git remote' [-v | --verbose] 'update' [-p | --prune] [( | )...]
@@ -155,6 +156,10 @@ With `--delete`, instead of changing existing URLs, all
URLs matching
regex are deleted for remote . Trying to delete all
non-push URLs is an error.
+
+With `--save-to-push`, the current URL is saved into the push URL before
+setting the URL to . Note that this command will not work if more than one
+URL is defined because the behavior would be ambiguous.
++
Note that the push URL and the fetch URL, even though they can
be set differently, must still refer to the same place. What you
pushed to the push URL should be what you would see if you
diff --git a/builtin/remote.c b/builtin/remote.c
index f7edf7f2cb..3249c3face 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -24,8 +24,9 @@ static const char * const builtin_remote_usage[] = {
N_("git remote set-branches [--add] ..."),
N_("git remote get-url [--push] [--all] "),
N_("git remote set-url [--push] []"),
- N_("git remote set-url --add "),
- N_("git remote set-url --delete "),
+ N_("git remote set-url --add [--push] "),
+ N_("git remote set-url --delete [--push] "),
+ N_("git remote set-url --save-to-push "),
NULL
};
@@ -77,8 +78,9 @@ static const char * const builtin_remote_geturl_usage[] = {
static const char * const builtin_remote_seturl_usage[] = {
N_("git remote set-url [--push] []"),
- N_("git remote set-url --add "),
- N_("git remote set-url --delete "),
+ N_("git remote set-url --add [--push] "),
+ N_("git remote set-url --delete [--push] "),
+ N_("git remote set-url --save-to-push "),
NULL
};
@@ -1519,7 +1521,7 @@ static int get_url(int argc, const char **argv)
static int set_url(int argc, const char **argv)
{
- int i, push_mode
[RFC PATCH v2] remote: add --save-push option to git remote set-url
This adds the --save-push option to `git remote set-url` such that when
executed, we move the remote.*.url to remote.*.pushurl and set
remote.*.url to the given url argument.
For example, if we have the following config:
[remote "origin"]
url = g...@github.com:git/git.git
`git remote set-url --save-push origin https://github.com/git/git.git`
would change the config to the following:
[remote "origin"]
url = https://github.com/git/git.git
pushurl = g...@github.com:git/git.git
Signed-off-by: Denton Liu
---
I decided to address your comments and reroll the patch one more time.
Even though the option isn't _that_ commonly used, I've managed to use
it a couple of times since I've implemented so I believe that this
option should be included.
---
Documentation/git-remote.txt | 5 +
builtin/remote.c | 25 -
2 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index 0cad37fb81..8ce85fe2f2 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -19,6 +19,7 @@ SYNOPSIS
'git remote set-url' [--push] []
'git remote set-url --add' [--push]
'git remote set-url --delete' [--push]
+'git remote set-url --save-push'
'git remote' [-v | --verbose] 'show' [-n] ...
'git remote prune' [-n | --dry-run] ...
'git remote' [-v | --verbose] 'update' [-p | --prune] [( | )...]
@@ -155,6 +156,10 @@ With `--delete`, instead of changing existing URLs, all
URLs matching
regex are deleted for remote . Trying to delete all
non-push URLs is an error.
+
+With `--save-push`, the current URL is saved into the push URL before setting
+the URL to . Note that this command will not work if more than one URL is
+defined or if any push URLs are defined because behavior would be ambiguous.
++
Note that the push URL and the fetch URL, even though they can
be set differently, must still refer to the same place. What you
pushed to the push URL should be what you would see if you
diff --git a/builtin/remote.c b/builtin/remote.c
index f7edf7f2cb..0eaec7ef38 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -24,8 +24,9 @@ static const char * const builtin_remote_usage[] = {
N_("git remote set-branches [--add] ..."),
N_("git remote get-url [--push] [--all] "),
N_("git remote set-url [--push] []"),
- N_("git remote set-url --add "),
- N_("git remote set-url --delete "),
+ N_("git remote set-url --add [--push] "),
+ N_("git remote set-url --delete [--push] "),
+ N_("git remote set-url --save-push "),
NULL
};
@@ -77,8 +78,9 @@ static const char * const builtin_remote_geturl_usage[] = {
static const char * const builtin_remote_seturl_usage[] = {
N_("git remote set-url [--push] []"),
- N_("git remote set-url --add "),
- N_("git remote set-url --delete "),
+ N_("git remote set-url --add [--push] "),
+ N_("git remote set-url --delete [--push] "),
+ N_("git remote set-url --save-push "),
NULL
};
@@ -1519,7 +1521,7 @@ static int get_url(int argc, const char **argv)
static int set_url(int argc, const char **argv)
{
- int i, push_mode = 0, add_mode = 0, delete_mode = 0;
+ int i, push_mode = 0, save_push = 0, add_mode = 0, delete_mode = 0;
int matches = 0, negative_matches = 0;
const char *remotename = NULL;
const char *newurl = NULL;
@@ -1532,6 +1534,8 @@ static int set_url(int argc, const char **argv)
struct option options[] = {
OPT_BOOL('\0', "push", _mode,
N_("manipulate push URLs")),
+ OPT_BOOL('\0', "save-push", _push,
+N_("change fetching URL behavior")),
OPT_BOOL('\0', "add", _mode,
N_("add URL")),
OPT_BOOL('\0', "delete", _mode,
@@ -1543,6 +1547,8 @@ static int set_url(int argc, const char **argv)
if (add_mode && delete_mode)
die(_("--add --delete doesn't make sense"));
+ if (save_push && (push_mode || add_mode || delete_mode))
+ die(_("--save-push cannot be used with other options"));
if (argc < 3 || argc > 4 || ((add_mode || delete_mode) && argc != 3))
usage_with_options(builtin_remote_seturl_usage, options);
@@ -1564,6 +1570,15 @@ static int set_url(int argc, const char **argv)
urlset = remote->pushurl;
urlset_nr = remote->pushurl_nr;
} else {
+ if (save_push) {
+ if (remote->url_
Re: [RFC PATCH 0/2] Fix scissors bug during merge conflict
On Wed, Nov 14, 2018 at 04:52:59PM +0900, Junio C Hamano wrote: > Denton Liu writes: > > > With this fix, the message becomes the following: > > > > Merge branch 'master' into new > > > > # >8 > > # Do not modify or remove the line above. > > # Everything below it will be ignored. > > # > > # Conflicts: > > # a > > I have a mixed feeling about this change and I certainly would not > call it a "fix". > > The reason why we give the list of conflicted paths that is > commented out is to remind the user of them in order to help them > describe what area of the codebase had overlapping changes, why, and > how the overlap was resolved, which is relevant information when > somebody else later needs to dig into the history to understand how > the code came into today's shape and why. For that reason, if a > careless user left conflicts list behind without describing these > details about the merge, it might be better to have the unexplained > list in the merge than nothing. > The reason why I implemented it this way is because the default cleanup setting (strip) produces this message: Merge branch 'master' into new # Conflicts: # a # # It looks like you may be committing a merge. # If this is not correct, please remove the file # .git/MERGE_HEAD # and try again. # Please enter the commit message for your changes. Lines starting # with '#' will be ignored, and an empty message aborts the commit. # # On branch new # All conflicts fixed but you are still merging. # # Changes to be committed: # modified: a # Which makes it seem like the `Conflicts:` section should be removed by default. > In theory, the above argument applies the same way for the paths to > be committed, but the list is fairly trivial to recreate with "git > diff $it^!", unlike "which paths had conflict", which can only be > found out by recreating the auto-merge. So in practice, the paths > that had conflicts is more worth showing than the paths that were > modified. > > So, I dunno. If we value the "more expensive list to reproduce", > the fix might be not to place it (and possibly the comments and > everything under the scissors line) behind a "# " comment char on > the line, without moving its position. If I understood correctly, then I have no strong opinions between uncommenting the Conflicts section by default and this change; I just think it'd be nice to have behaviour that's consistent. > > . > >
[RFC PATCH 0/2] Fix scissors bug during merge conflict
I discovered a bug in Git a while ago where if one is using commit.cleanup = scissors, if making a commit after a merge conflict, the scissors line will be placed after the `Conflicts:` section. As a result, a careless Git user (e.g. me) may accidentally commit the `Conflicts:` section. Here is a test case to replicate the behaviour: mkdir test cd test/ git init git config commit.cleanup scissors touch a git add a git commit -m 'test' echo a > a git commit -am 'test2' git checkout -b new HEAD^ echo b > a git commit -am 'test3' git merge master echo c > a git add a git commit # look at the commit message here And the resulting message that's sent to the text editor: Merge branch 'master' into new # Conflicts: # a # >8 # Do not modify or remove the line above. # Everything below it will be ignored. # # It looks like you may be committing a merge. # If this is not correct, please remove the file # .git/MERGE_HEAD # and try again. # Please enter the commit message for your changes. Lines starting # with '#' will be kept; you may remove them yourself if you want to. # An empty message aborts the commit. # # On branch new # All conflicts fixed but you are still merging. # # Changes to be committed: # modified: a # With this fix, the message becomes the following: Merge branch 'master' into new # >8 # Do not modify or remove the line above. # Everything below it will be ignored. # # Conflicts: # a # # It looks like you may be committing a merge. # If this is not correct, please remove the file # .git/MERGE_HEAD # and try again. # Please enter the commit message for your changes. Lines starting # with '#' will be kept; you may remove them yourself if you want to. # An empty message aborts the commit. # # On branch new # All conflicts fixed but you are still merging. # # Changes to be committed: # modified: a # Let me know what you think of the change. Of course, documentation and testing will come after this leaves the RFC phase. Denton Liu (2): commit: don't add scissors line if one exists merge: add scissors line on merge conflict builtin/commit.c | 11 +-- builtin/merge.c | 16 2 files changed, 25 insertions(+), 2 deletions(-) -- 2.19.1
[RFC PATCH 1/2] commit: don't add scissors line if one exists
If commit.cleanup = scissors is specified, don't produce a scissors line
if one already exists in the commit message.
Signed-off-by: Denton Liu
---
builtin/commit.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 0d9828e29e..e486246329 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -659,6 +659,7 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
const char *hook_arg2 = NULL;
int clean_message_contents = (cleanup_mode != COMMIT_MSG_CLEANUP_NONE);
int old_display_comment_prefix;
+ int contains_scissors = 0;
/* This checks and barfs if author is badly specified */
determine_author_info(author_ident);
@@ -742,6 +743,10 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
clean_message_contents = 0;
}
+ if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&
+ wt_status_locate_end(sb.buf, sb.len) != sb.len)
+ contains_scissors = 1;
+
/*
* The remaining cases don't modify the template message, but
* just set the argument(s) to the prepare-commit-msg hook.
@@ -798,7 +803,8 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
struct ident_split ci, ai;
if (whence != FROM_COMMIT) {
- if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)
+ if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&
+ !contains_scissors)
wt_status_add_cut_line(s->fp);
status_printf_ln(s, GIT_COLOR_NORMAL,
whence == FROM_MERGE
@@ -824,7 +830,8 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
" Lines starting\nwith '%c' will be ignored,
and an empty"
" message aborts the commit.\n"),
comment_line_char);
else if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&
-whence == FROM_COMMIT)
+whence == FROM_COMMIT &&
+!contains_scissors)
wt_status_add_cut_line(s->fp);
else /* COMMIT_MSG_CLEANUP_SPACE, that is. */
status_printf(s, GIT_COLOR_NORMAL,
--
2.19.1
[RFC PATCH 2/2] merge: add scissors line on merge conflict
This fixes a bug where the scissors line is placed after the Conflicts:
section, in the case where a merge conflict occurs and
merge.cleanup = scissors.
Signed-off-by: Denton Liu
---
builtin/merge.c | 16
1 file changed, 16 insertions(+)
diff --git a/builtin/merge.c b/builtin/merge.c
index 8f4a5065c2..c5010cee5e 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -36,6 +36,7 @@
#include "packfile.h"
#include "tag.h"
#include "alias.h"
+#include "wt-status.h"
#define DEFAULT_TWOHEAD (1<<0)
#define DEFAULT_OCTOPUS (1<<1)
@@ -96,6 +97,9 @@ enum ff_type {
static enum ff_type fast_forward = FF_ALLOW;
+const char *cleanup_arg;
+int put_scissors;
+
static int option_parse_message(const struct option *opt,
const char *arg, int unset)
{
@@ -243,6 +247,7 @@ static struct option builtin_merge_options[] = {
N_("perform a commit if the merge succeeds (default)")),
OPT_BOOL('e', "edit", _edit,
N_("edit message before committing")),
+ OPT_STRING(0, "cleanup", _arg, N_("default"), N_("how to strip
spaces and #comments from message")),
OPT_SET_INT(0, "ff", _forward, N_("allow fast-forward (default)"),
FF_ALLOW),
OPT_SET_INT_F(0, "ff-only", _forward,
N_("abort if fast-forward is not possible"),
@@ -606,6 +611,8 @@ static int git_merge_config(const char *k, const char *v,
void *cb)
return git_config_string(_twohead, k, v);
else if (!strcmp(k, "pull.octopus"))
return git_config_string(_octopus, k, v);
+ else if (!strcmp(k, "commit.cleanup"))
+ return git_config_string(_arg, k, v);
else if (!strcmp(k, "merge.renormalize"))
option_renormalize = git_config_bool(k, v);
else if (!strcmp(k, "merge.ff")) {
@@ -894,6 +901,13 @@ static int suggest_conflicts(void)
filename = git_path_merge_msg(the_repository);
fp = xfopen(filename, "a");
+ if (put_scissors) {
+ fputc('\n', fp);
+ wt_status_add_cut_line(fp);
+ /* comments out the newline from append_conflicts_hint */
+ fputc(comment_line_char, fp);
+ }
+
append_conflicts_hint();
fputs(msgbuf.buf, fp);
strbuf_release();
@@ -1402,6 +1416,8 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
if (option_edit < 0)
option_edit = default_edit_option();
+ put_scissors = cleanup_arg && !strcmp(cleanup_arg, "scissors");
+
if (!use_strategies) {
if (!remoteheads)
; /* already up-to-date */
--
2.19.1
Re: [RFC PATCH 1/2] commit: don't add scissors line if one exists
On Wed, Nov 14, 2018 at 05:06:32PM +0900, Junio C Hamano wrote: > Denton Liu writes: > > > If commit.cleanup = scissors is specified, don't produce a scissors line > > if one already exists in the commit message. > > It is good that you won't have two such lines in the end result, but > is this (1) hiding real problem under the rug? (2) losing information? > > If the current invocation of "git commit" added a scissors line in > the buffer to be edited already, and we are adding another one in > this function, is it possible that the real problem that somebody > else has called wt_status_add_cut_line() before this function is > called, in which case that other caller is what we need to fix, > instead of this one? > In patch 2/2, I intentionally inserted a scissors line into MERGE_MSG so this patch ensures that we don't get duplicate scissors. > If the existing line in the buffer came from the end user (perhaps > it was given from "-F ", etc., with "-e" option) or --amend, > how can we be sure if it is OK to lose everything after that > scissors looking line? In other words, the scissors looking line > may just be part of the log message, in which case we may want to > quote/escape it, so that the true scissors we append at a later > place in the buffer would be noticed without losing the text before > and after that scissors looking line we already had when this > function was called? > With the existing behaviour, any messages that contain a scissors looking line will get cut at the earliest scissors anyway, so I believe that this patch would not change the behaviour. If the users were dealing with commit messages with a scissors looking line, the current behaviour already requires users to be extra careful to ensure that the scissors don't get accidentally removed so in the interest of preserving the existing behaviour, I don't think that any extra information would be lost from this patch.
[PATCH v2 2/2] merge: add scissors line on merge conflict
This fixes a bug where the scissors line is placed after the Conflicts:
section, in the case where a merge conflict occurs and
merge.cleanup = scissors.
In addition, we give pull the passthrough option of --cleanup so that it
can also take advantage of this change.
Signed-off-by: Denton Liu
---
Documentation/merge-options.txt | 6 +
builtin/merge.c | 16 +++
builtin/pull.c | 6 +
t/t7600-merge.sh| 48 +
4 files changed, 76 insertions(+)
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 63a3fc0954..115e0ca6f0 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -27,6 +27,12 @@ they run `git merge`. To make it easier to adjust such
scripts to the
updated behaviour, the environment variable `GIT_MERGE_AUTOEDIT` can be
set to `no` at the beginning of them.
+--cleanup=::
+ This option determines how the merge message will be cleaned up
+ before being passed on. Specifically, if the '' is given a
+ value of `scissors`, scissors will be prepended to the message in
+ the case of a merge conflict. See also linkgit:git-commit[1].
+
--ff::
When the merge resolves as a fast-forward, only update the branch
pointer, without creating a merge commit. This is the default
diff --git a/builtin/merge.c b/builtin/merge.c
index 8f4a5065c2..23a6e6bb93 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -36,6 +36,7 @@
#include "packfile.h"
#include "tag.h"
#include "alias.h"
+#include "wt-status.h"
#define DEFAULT_TWOHEAD (1<<0)
#define DEFAULT_OCTOPUS (1<<1)
@@ -96,6 +97,9 @@ enum ff_type {
static enum ff_type fast_forward = FF_ALLOW;
+static const char *cleanup_arg;
+static int put_scissors;
+
static int option_parse_message(const struct option *opt,
const char *arg, int unset)
{
@@ -243,6 +247,7 @@ static struct option builtin_merge_options[] = {
N_("perform a commit if the merge succeeds (default)")),
OPT_BOOL('e', "edit", _edit,
N_("edit message before committing")),
+ OPT_STRING(0, "cleanup", _arg, N_("default"), N_("how to strip
spaces and #comments from message")),
OPT_SET_INT(0, "ff", _forward, N_("allow fast-forward (default)"),
FF_ALLOW),
OPT_SET_INT_F(0, "ff-only", _forward,
N_("abort if fast-forward is not possible"),
@@ -606,6 +611,8 @@ static int git_merge_config(const char *k, const char *v,
void *cb)
return git_config_string(_twohead, k, v);
else if (!strcmp(k, "pull.octopus"))
return git_config_string(_octopus, k, v);
+ else if (!strcmp(k, "commit.cleanup"))
+ return git_config_string(_arg, k, v);
else if (!strcmp(k, "merge.renormalize"))
option_renormalize = git_config_bool(k, v);
else if (!strcmp(k, "merge.ff")) {
@@ -894,6 +901,13 @@ static int suggest_conflicts(void)
filename = git_path_merge_msg(the_repository);
fp = xfopen(filename, "a");
+ if (put_scissors) {
+ fputc('\n', fp);
+ wt_status_add_cut_line(fp);
+ /* comments out the newline from append_conflicts_hint */
+ fputc(comment_line_char, fp);
+ }
+
append_conflicts_hint();
fputs(msgbuf.buf, fp);
strbuf_release();
@@ -1402,6 +1416,8 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
if (option_edit < 0)
option_edit = default_edit_option();
+ put_scissors = cleanup_arg && !strcmp(cleanup_arg, "scissors");
+
if (!use_strategies) {
if (!remoteheads)
; /* already up-to-date */
diff --git a/builtin/pull.c b/builtin/pull.c
index 681c127a07..88245bce0e 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -95,6 +95,7 @@ static char *opt_signoff;
static char *opt_squash;
static char *opt_commit;
static char *opt_edit;
+static char *opt_cleanup;
static char *opt_ff;
static char *opt_verify_signatures;
static int opt_autostash = -1;
@@ -162,6 +163,9 @@ static struct option pull_options[] = {
OPT_PASSTHRU(0, "edit", _edit, NULL,
N_("edit message before committing"),
PARSE_OPT_NOARG),
+ OPT_PASSTHRU(0, "cleanup", _cleanup, NULL,
+ N_("how to strip spaces and #comments from message"),
+ PARSE_OPT_NOARG),
OPT_PASSTHRU(0, "ff", _ff, NULL,
N_("allow fast-forward"),
PARSE_OPT_NOARG),
@@ -625,6 +629,8 @@ static int run_merge(void)
[PATCH v4 2/2] merge: add scissors line on merge conflict
This fixes a bug where the scissors line is placed after the Conflicts:
section, in the case where a merge conflict occurs and
merge.cleanup = scissors.
Next, if commit.cleanup = scissors is specified, don't produce a
scissors line in commit if one already exists in the MERGE_MSG file.
Finally, we give pull the passthrough option of --cleanup so that it
can also take advantage of this change.
Signed-off-by: Denton Liu
---
Documentation/merge-options.txt | 6 +
builtin/commit.c| 20 ++
builtin/merge.c | 16
builtin/pull.c | 6 +
t/t7600-merge.sh| 46 +
5 files changed, 89 insertions(+), 5 deletions(-)
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 63a3fc0954..115e0ca6f0 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -27,6 +27,12 @@ they run `git merge`. To make it easier to adjust such
scripts to the
updated behaviour, the environment variable `GIT_MERGE_AUTOEDIT` can be
set to `no` at the beginning of them.
+--cleanup=::
+ This option determines how the merge message will be cleaned up
+ before being passed on. Specifically, if the '' is given a
+ value of `scissors`, scissors will be prepended to the message in
+ the case of a merge conflict. See also linkgit:git-commit[1].
+
--ff::
When the merge resolves as a fast-forward, only update the branch
pointer, without creating a merge commit. This is the default
diff --git a/builtin/commit.c b/builtin/commit.c
index 0d9828e29e..7902645bc9 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -659,6 +659,7 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
const char *hook_arg2 = NULL;
int clean_message_contents = (cleanup_mode != COMMIT_MSG_CLEANUP_NONE);
int old_display_comment_prefix;
+ int merge_contains_scissors = 0;
/* This checks and barfs if author is badly specified */
determine_author_info(author_ident);
@@ -719,6 +720,8 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
strbuf_addbuf(, );
hook_arg1 = "message";
} else if (!stat(git_path_merge_msg(the_repository), )) {
+ size_t merge_msg_start;
+
/*
* prepend SQUASH_MSG here if it exists and a
* "merge --squash" was originally performed
@@ -729,8 +732,14 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
hook_arg1 = "squash";
} else
hook_arg1 = "merge";
+
+ merge_msg_start = sb.len;
if (strbuf_read_file(, git_path_merge_msg(the_repository),
0) < 0)
die_errno(_("could not read MERGE_MSG"));
+
+ if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&
+ wt_status_locate_end(sb.buf + merge_msg_start, sb.len -
merge_msg_start) < sb.len - merge_msg_start)
+ merge_contains_scissors = 1;
} else if (!stat(git_path_squash_msg(the_repository), )) {
if (strbuf_read_file(, git_path_squash_msg(the_repository),
0) < 0)
die_errno(_("could not read SQUASH_MSG"));
@@ -798,7 +807,8 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
struct ident_split ci, ai;
if (whence != FROM_COMMIT) {
- if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)
+ if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&
+ !merge_contains_scissors)
wt_status_add_cut_line(s->fp);
status_printf_ln(s, GIT_COLOR_NORMAL,
whence == FROM_MERGE
@@ -823,10 +833,10 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
_("Please enter the commit message for your
changes."
" Lines starting\nwith '%c' will be ignored,
and an empty"
" message aborts the commit.\n"),
comment_line_char);
- else if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&
-whence == FROM_COMMIT)
- wt_status_add_cut_line(s->fp);
- else /* COMMIT_MSG_CLEANUP_SPACE, that is. */
+ else if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) {
+ if (whence == FROM_COMMIT && !merge_contains_scissors)
+ wt_status_add_cut_line(s->fp);
+ } else /* COMMIT_MSG_CLEANUP_SPAC
[PATCH v4 1/2] t7600: clean up 'merge --squash c3 with c7' test
This cleans up the original test by removing some unnecessary braces and
removing a pipe.
Helped-by: SZEDER Gábor
Signed-off-by: Denton Liu
---
t/t7600-merge.sh | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 106148254d..d879efd330 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -233,8 +233,7 @@ test_expect_success 'merge --squash c3 with c7' '
cat result.9z >file &&
git commit --no-edit -a &&
- {
- cat <<-EOF
+ cat >expect <<-EOF &&
Squashed commit of the following:
$(git show -s c7)
@@ -242,8 +241,8 @@ test_expect_success 'merge --squash c3 with c7' '
# Conflicts:
# file
EOF
- } >expect &&
- git cat-file commit HEAD | sed -e '1,/^$/d' >actual &&
+ git cat-file commit HEAD >tmp &&
+ sed -e '1,/^$/d' actual &&
test_cmp expect actual
'
--
2.19.1
[PATCH v4 0/2] Fix scissors bug during merge conflict
Thanks for catching that, Szeder. I tested this revised patch under GETTEXT_POISON and it should be passing tests now. Changes since V1: * Only check MERGE_MSG for a scissors line instead of all prepended files * Make a variable static in merge where appropriate * Add passthrough options in pull * Add documentation for the new option * Add tests to ensure desired behaviour Changes since V2: * Merge both patches into one patch * Fix bug in help message printing logic Changes since V3: * Add patch to cleanup 'merge --squash c3 with c7' test in t7600 * Use test_i18ncmp instead of test_cmp to pass GETTEXT_POISON tests Denton Liu (2): t7600: clean up 'merge --squash c3 with c7' test merge: add scissors line on merge conflict Documentation/merge-options.txt | 6 builtin/commit.c| 20 + builtin/merge.c | 16 ++ builtin/pull.c | 6 t/t7600-merge.sh| 53 ++--- 5 files changed, 92 insertions(+), 9 deletions(-) -- 2.19.1
Re: [PATCH v4 0/2] Fix scissors bug during merge conflict
On Wed, Nov 21, 2018 at 06:38:20PM +0900, Junio C Hamano wrote: > Denton Liu writes: > > > Changes since V3: > > * Add patch to cleanup 'merge --squash c3 with c7' test in t7600 > > * Use test_i18ncmp instead of test_cmp to pass GETTEXT_POISON tests > > Queued. Thanks, both. I just realised that there is a slight problem with the proposed change. When we do a merge and there are no merge conflicts, at the end of the merge, we get dropped into an editor with this text: Merge branch 'master' into new # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit. Note that in git-merge, the cleanup only removes commented lines and this cannot be configured to be scissors or whatever else. I think that the fact that it's not configurable isn't a problem; most hardcore commit message editing happens in git-commit anyway. However, since we taught git-merge the --cleanup option, this might be misleading for the end-user since they would expect the MERGE_MSG to be cleaned up as specified. I see two resolutions for this. We can either rename --cleanup more precisely so users won't be confused (perhaps something like --merge-conflict-scissors but a lot more snappy) or we can actually make git-merge respect the cleanup option and post-process the message according to the specified cleanup rule. I would personally think the first option is better than the second but I'd like to hear your thoughts. Thanks!
[PATCH v2 0/2] Fix scissors bug during merge conflict
Thanks for your feedback, Junio. I tried to reroll the patch by adding another option into the MERGE_MODE file but unfortunately, it didn't work completely because doing `merge --squash` doesn't produce a MERGE_MODE. In addition to this, because of the way merge and commit were structured, I needed to reorder a lot of calls because some variables were only being set after I needed them. Unless we want to produce a MERGE_MODE during --squash (which I don't think is worth it) I don't think that this is the way to go. Instead, I just refined my first approach and only checked the contents of MERGE_MSG for a scissors line. The MERGE_MSG is going to be machine-generated anyway so we should be safe from accidentally ignoring a human-placed scissors line. Changes since V1: - * Only check MERGE_MSG for a scissors line instead of all prepended files * Make a variable static in merge where appropriate * Add passthrough options in pull * Add documentation for the new option * Add tests to ensure desired behaviour Denton Liu (2): commit: don't add scissors line if one exists merge: add scissors line on merge conflict Documentation/merge-options.txt | 6 + builtin/commit.c| 15 +-- builtin/merge.c | 16 +++ builtin/pull.c | 6 + t/t7600-merge.sh| 48 + 5 files changed, 89 insertions(+), 2 deletions(-) -- 2.19.1
[PATCH v2 1/2] commit: don't add scissors line if one exists in MERGE_MSG
If commit.cleanup = scissors is specified, don't produce a scissors line
if one already exists in the MERGE_MSG file.
Signed-off-by: Denton Liu
---
builtin/commit.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 0d9828e29e..a74a324b7a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -659,6 +659,7 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
const char *hook_arg2 = NULL;
int clean_message_contents = (cleanup_mode != COMMIT_MSG_CLEANUP_NONE);
int old_display_comment_prefix;
+ int merge_contains_scissors = 0;
/* This checks and barfs if author is badly specified */
determine_author_info(author_ident);
@@ -719,6 +720,8 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
strbuf_addbuf(, );
hook_arg1 = "message";
} else if (!stat(git_path_merge_msg(the_repository), )) {
+ size_t merge_msg_start;
+
/*
* prepend SQUASH_MSG here if it exists and a
* "merge --squash" was originally performed
@@ -729,8 +732,14 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
hook_arg1 = "squash";
} else
hook_arg1 = "merge";
+
+ merge_msg_start = sb.len;
if (strbuf_read_file(, git_path_merge_msg(the_repository),
0) < 0)
die_errno(_("could not read MERGE_MSG"));
+
+ if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&
+ wt_status_locate_end(sb.buf + merge_msg_start, sb.len -
merge_msg_start) < sb.len - merge_msg_start)
+ merge_contains_scissors = 1;
} else if (!stat(git_path_squash_msg(the_repository), )) {
if (strbuf_read_file(, git_path_squash_msg(the_repository),
0) < 0)
die_errno(_("could not read SQUASH_MSG"));
@@ -798,7 +807,8 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
struct ident_split ci, ai;
if (whence != FROM_COMMIT) {
- if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)
+ if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&
+ !merge_contains_scissors)
wt_status_add_cut_line(s->fp);
status_printf_ln(s, GIT_COLOR_NORMAL,
whence == FROM_MERGE
@@ -824,7 +834,8 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
" Lines starting\nwith '%c' will be ignored,
and an empty"
" message aborts the commit.\n"),
comment_line_char);
else if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&
-whence == FROM_COMMIT)
+whence == FROM_COMMIT &&
+!merge_contains_scissors)
wt_status_add_cut_line(s->fp);
else /* COMMIT_MSG_CLEANUP_SPACE, that is. */
status_printf(s, GIT_COLOR_NORMAL,
--
2.19.1
[PATCH v3 1/1] merge: add scissors line on merge conflict
This fixes a bug where the scissors line is placed after the Conflicts:
section, in the case where a merge conflict occurs and
merge.cleanup = scissors.
Next, if commit.cleanup = scissors is specified, don't produce a
scissors line in commit if one already exists in the MERGE_MSG file.
Finally, we give pull the passthrough option of --cleanup so that it
can also take advantage of this change.
Signed-off-by: Denton Liu
---
Documentation/merge-options.txt | 6 +
builtin/commit.c| 20 ++
builtin/merge.c | 16 +++
builtin/pull.c | 6 +
t/t7600-merge.sh| 48 +
5 files changed, 91 insertions(+), 5 deletions(-)
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 63a3fc0954..115e0ca6f0 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -27,6 +27,12 @@ they run `git merge`. To make it easier to adjust such
scripts to the
updated behaviour, the environment variable `GIT_MERGE_AUTOEDIT` can be
set to `no` at the beginning of them.
+--cleanup=::
+ This option determines how the merge message will be cleaned up
+ before being passed on. Specifically, if the '' is given a
+ value of `scissors`, scissors will be prepended to the message in
+ the case of a merge conflict. See also linkgit:git-commit[1].
+
--ff::
When the merge resolves as a fast-forward, only update the branch
pointer, without creating a merge commit. This is the default
diff --git a/builtin/commit.c b/builtin/commit.c
index 0d9828e29e..7902645bc9 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -659,6 +659,7 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
const char *hook_arg2 = NULL;
int clean_message_contents = (cleanup_mode != COMMIT_MSG_CLEANUP_NONE);
int old_display_comment_prefix;
+ int merge_contains_scissors = 0;
/* This checks and barfs if author is badly specified */
determine_author_info(author_ident);
@@ -719,6 +720,8 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
strbuf_addbuf(, );
hook_arg1 = "message";
} else if (!stat(git_path_merge_msg(the_repository), )) {
+ size_t merge_msg_start;
+
/*
* prepend SQUASH_MSG here if it exists and a
* "merge --squash" was originally performed
@@ -729,8 +732,14 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
hook_arg1 = "squash";
} else
hook_arg1 = "merge";
+
+ merge_msg_start = sb.len;
if (strbuf_read_file(, git_path_merge_msg(the_repository),
0) < 0)
die_errno(_("could not read MERGE_MSG"));
+
+ if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&
+ wt_status_locate_end(sb.buf + merge_msg_start, sb.len -
merge_msg_start) < sb.len - merge_msg_start)
+ merge_contains_scissors = 1;
} else if (!stat(git_path_squash_msg(the_repository), )) {
if (strbuf_read_file(, git_path_squash_msg(the_repository),
0) < 0)
die_errno(_("could not read SQUASH_MSG"));
@@ -798,7 +807,8 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
struct ident_split ci, ai;
if (whence != FROM_COMMIT) {
- if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)
+ if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&
+ !merge_contains_scissors)
wt_status_add_cut_line(s->fp);
status_printf_ln(s, GIT_COLOR_NORMAL,
whence == FROM_MERGE
@@ -823,10 +833,10 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
_("Please enter the commit message for your
changes."
" Lines starting\nwith '%c' will be ignored,
and an empty"
" message aborts the commit.\n"),
comment_line_char);
- else if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&
-whence == FROM_COMMIT)
- wt_status_add_cut_line(s->fp);
- else /* COMMIT_MSG_CLEANUP_SPACE, that is. */
+ else if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS) {
+ if (whence == FROM_COMMIT && !merge_contains_scissors)
+ wt_status_add_cut_line(s->fp);
+ } else /* COMMIT_MSG_CLEANUP_SPAC
[PATCH v3 0/1] Fix scissors bug during merge conflict
On Sat, Nov 17, 2018 at 05:06:43PM +0900, Junio C Hamano wrote:
> Are we already sometimes adding a scissors line in that file? The
> impression I was getting was that the change in the step 2/2 is the
> only reason why this becomes necessary. In which case this change
> makes no sense as a standalone patch and requires 2/2 to be a
> sensible change, no?
>
My mistake, I guess I went a little overboard trying to split my
contribution into digestable patches.
> > @@ -798,7 +807,8 @@ static int prepare_to_commit(const char *index_file,
> > const char *prefix,
> > struct ident_split ci, ai;
> >
> > if (whence != FROM_COMMIT) {
> > - if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)
> > + if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&
> > + !merge_contains_scissors)
> > wt_status_add_cut_line(s->fp);
> > status_printf_ln(s, GIT_COLOR_NORMAL,
> > whence == FROM_MERGE
>
> This one is done before we show a block of text, which looks correct.
>
> > @@ -824,7 +834,8 @@ static int prepare_to_commit(const char *index_file,
> > const char *prefix,
> > " Lines starting\nwith '%c' will be ignored,
> > and an empty"
> > " message aborts the commit.\n"),
> > comment_line_char);
> > else if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&
> > -whence == FROM_COMMIT)
> > +whence == FROM_COMMIT &&
> > +!merge_contains_scissors)
> > wt_status_add_cut_line(s->fp);
> > else /* COMMIT_MSG_CLEANUP_SPACE, that is. */
> > status_printf(s, GIT_COLOR_NORMAL,
>
> The correctness of this one in an if/elseif/else cascade is less
> clear. When "contains scissors" logic does not kick in, we just
> have a scissors line here (i.e. the original behaviour). Now when
> the new logic kicks in, we not just omit the (extra) scissors line,
> but also say "Please enter the commit message..." which is the
> message used with the "comment line char" mode of the clean-up.
>
> I wonder if this is what you really meant to have instead:
>
> > else if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&
> > -whence == FROM_COMMIT)
> > - wt_status_add_cut_line(s->fp);
> > +whence == FROM_COMMIT) {
> > +if (!merge_contains_scissors)
> > + wt_status_add_cut_line(s->fp);
> > + }
> > else /* COMMIT_MSG_CLEANUP_SPACE, that is. */
> > status_printf(s, GIT_COLOR_NORMAL,
>
> That is, the only behaviour change in "merge contains scissors"
> mode is to omit the cut line and nothing else.
Thanks for catching this. I noticed that the originally behaviour is
buggy too: in the case where we're merging a commit and scissors are
printed from the `if (whence != FROM_COMMIT)` block, the original
behaviour would drop us into the else (COMMIT_MSG_CLEANUP_SPACE)
statement, which is undesired. With this fix, the message about `#`
_not_ being removed is now silenced in both cases.
Changes since V1:
* Only check MERGE_MSG for a scissors line instead of all prepended
files
* Make a variable static in merge where appropriate
* Add passthrough options in pull
* Add documentation for the new option
* Add tests to ensure desired behaviour
Changes since V2:
* Merge both patches into one patch
* Fix bug in help message printing logic
Denton Liu (1):
merge: add scissors line on merge conflict
Documentation/merge-options.txt | 6 +
builtin/commit.c| 20 ++
builtin/merge.c | 16 +++
builtin/pull.c | 6 +
t/t7600-merge.sh| 48 +
5 files changed, 91 insertions(+), 5 deletions(-)
--
2.19.1
