Git development
 help / color / mirror / Atom feed
* Re: feature request: add -q to "git branch"
From: Pranit Bauva @ 2017-02-04 21:17 UTC (permalink / raw)
  To: Kevin Layer; +Cc: Git List
In-Reply-To: <CAGSZTjLmYCyKZ1BBRv+JVYq4oX7EQcNzyxAnS_3NBUPjr3g8zQ@mail.gmail.com>

Hey Kevin,

On Fri, Feb 3, 2017 at 11:59 PM, Kevin Layer <layer@known.net> wrote:
> It should be possible to quietly create a branch.
>
> Thanks.
>
> Kevin

^ permalink raw reply

* Re: [PATCH v3] parse-remote: remove reference to unused op_prep
From: Pranit Bauva @ 2017-02-04 21:15 UTC (permalink / raw)
  To: Siddharth Kannan; +Cc: Git List
In-Reply-To: <1486218663-31820-1-git-send-email-kannan.siddharth12@gmail.com>

Hey Siddharth,

On Sat, Feb 4, 2017 at 8:01 PM, Siddharth Kannan
<kannan.siddharth12@gmail.com> wrote:
> The error_on_missing_default_upstream helper function learned to
> take op_prep argument with 15a147e618 ("rebase: use @{upstream}
> if no upstream specified", 2011-02-09), but as of 045fac5845
> ("i18n: git-parse-remote.sh: mark strings for translation",
>  2016-04-19), the argument is no longer used.  Remove it.
>
> Signed-off-by: Siddharth Kannan <kannan.siddharth12@gmail.com>

This looks good to me! Thanks :)

Regards,
Pranit Bauva

^ permalink raw reply

* Re: git push failing when push.recurseSubmodules on-demand and git commit --amend was used in submodule.
From: Stefan Beller @ 2017-02-04 18:43 UTC (permalink / raw)
  To: Carlo Wood; +Cc: Junio C Hamano, Heiko Voigt, git@vger.kernel.org
In-Reply-To: <20170201021040.4f6cc827@hikaru>

On Tue, Jan 31, 2017 at 5:10 PM, Carlo Wood <carlo@alinoe.com> wrote:
> On Tue, 31 Jan 2017 14:08:41 -0800
> Stefan Beller <sbeller@google.com> wrote:
>
>> On Sun, Jan 29, 2017 at 5:00 PM, Junio C Hamano <gitster@pobox.com>
>> wrote:
>> >  2. If the amend is good and ready to go, "git add" to update the
>> >     superproject to make that amended result the one that is needed
>> >     in the submodule.
>>
>> yup.
>
> But that is what I am doing. The amended commit IS already
> added to the superproject (and pushed to the remote).
>
> Please have a look at my script, this happens here:
>
> # Correct that in the parent too:
> pushd parent
> git add subm
> git commit -m 'Updated subm.'
> popd

And if you were to use ammend here, too; there would be no problem;
In the parent there are now two commits, the first one pointing at the
first (unammended) commit in the submodule, the second pointing to
the corrected commit.

>
> The commit from before the amend was added to the super
> project (but never pushed) but has now been completely
> replaced. I still think this is a flaw in git. It shouldn't
> not complain and simply push.

The problem here is in the design.
When "on-demand" is set, the parent repo determines which
sumbodules needs pushing, then runs a plain "git push" in them
and then checks again.

The push operation in the submodule did not push out the un-amended
commit (which is correct IMO).

The parent in the second check determines that there is a commit, that
is not pushed, though. Maybe we need an option there "on-demand-but-no-recheck"
as a weaker promise what Git can deliver there.

>
> --
> Carlo Wood <carlo@alinoe.com>

^ permalink raw reply

* Re: Git clonebundles
From: Shawn Pearce @ 2017-02-04 17:39 UTC (permalink / raw)
  To: Stefan Saasen; +Cc: Git Mailing List
In-Reply-To: <CADoxLGPFgF7W4XJzt0X+xFJDoN6RmfFGx_96MO9GPSSOjDK0EQ@mail.gmail.com>

On Mon, Jan 30, 2017 at 11:00 PM, Stefan Saasen <ssaasen@atlassian.com> wrote:
>
> Bitbucket recently added support for Mercurial’s clonebundle extension
> (http://gregoryszorc.com/blog/2015/10/22/cloning-improvements-in-mercurial-3.6/).
> Mercurial’s clone bundles allow the Mercurial client to seed a repository using
> a bundle file instead of dynamically generating a bundle for the client.
...
> Prior art
> ~~~~~~~~~
>
> Our proof-of-concept is built on top of ideas that have been
> circulating for a while. We are aware of a number of proposed changes
> in this space:
>
>
> * Jeff King's work on network bundles:
> https://github.com/peff/git/commit/17e2409df37edd0c49ef7d35f47a7695f9608900
> * Nguyễn Thái Ngọc Duy's work on "[PATCH 0/8] Resumable clone
> revisited, proof of concept":
> https://www.spinics.net/lists/git/msg267260.html
> * Resumable clone work by Kevin Wern:
> https://public-inbox.org/git/1473984742-12516-1-git-send-email-kevin.m.wern@gmail.com/

I think you missed the most common deployment of prior art, which is
Android using the git-repo tool[1]. The git-repo tool has had
clone.bundle support since Sep 2011[2] and the Android Git servers
have been answering /clone.bundle requests[3] since just before that.
The bundle files are generated with `git bundle create` on a regular
schedule by cron.

[1] https://gerrit.googlesource.com/git-repo/+/04071c1c72437a930db017bd4c562ad06087986a/project.py#2091
[2] https://gerrit.googlesource.com/git-repo/+/f322b9abb4cadc67b991baf6ba1b9f2fbd5d7812
[3] https://android.googlesource.com/platform/frameworks/base/clone.bundle


> Whilst the above mentioned proposals/proposed changes are in a similar
> space, I would be interest to understand whether there is any
> consensus on the general idea of supporting static bundle files as a
> mechanism to seed a repository?

I don't think we have a consensus on how to advertise a bundle file is
available, which is why there are so many instances of prior art. In
2011 I just threw together /clone.bundle on HTTP because it was easy
to make the Python wrapper ask for the file and handle 404 gracefully
as not found and fall back to `git clone`.

^ permalink raw reply

* git-gui ignores core.hookspath
From: matthias.serfling @ 2017-02-04 17:14 UTC (permalink / raw)
  To: git

Hello,
I’m running on 

$ git --version --build-options
git version 2.11.0.windows.3
built from commit: e11df2efb3072fe73153442589129d2eb8d9ea02
sizeof-long: 4
machine: x86_64


and trying to use core.hookspath with git-gui in my local repository in windows cmd shell 
$ cmd.exe /c ver
Microsoft Windows [Version 6.1.7601]
I have defined a pre-commit hook and set the path to it using core.hookspath, but when commiting in git gui it is not considered. 


Commiting directly on the cmd shell using git commit -a -m "ndkfnj" it works perfectly.

Thanks for any advice or bugfix

^ permalink raw reply

* [PATCH v3] parse-remote: remove reference to unused op_prep
From: Siddharth Kannan @ 2017-02-04 14:31 UTC (permalink / raw)
  To: git; +Cc: pranit.bauva, Siddharth Kannan

The error_on_missing_default_upstream helper function learned to
take op_prep argument with 15a147e618 ("rebase: use @{upstream}
if no upstream specified", 2011-02-09), but as of 045fac5845
("i18n: git-parse-remote.sh: mark strings for translation",
 2016-04-19), the argument is no longer used.  Remove it.

Signed-off-by: Siddharth Kannan <kannan.siddharth12@gmail.com>
---
Thanks a lot for the review, Pranit and Junio! I have made the appropriate
changes, and the edit to the file inside contrib/examples/ has been removed from
this patch.

 git-parse-remote.sh | 3 +--
 git-rebase.sh       | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/git-parse-remote.sh b/git-parse-remote.sh
index d3c3998..9698a05 100644
--- a/git-parse-remote.sh
+++ b/git-parse-remote.sh
@@ -56,8 +56,7 @@ get_remote_merge_branch () {
 error_on_missing_default_upstream () {
 	cmd="$1"
 	op_type="$2"
-	op_prep="$3" # FIXME: op_prep is no longer used
-	example="$4"
+	example="$3"
 	branch_name=$(git symbolic-ref -q HEAD)
 	display_branch_name="${branch_name#refs/heads/}"
 	# If there's only one remote, use that in the suggestion
diff --git a/git-rebase.sh b/git-rebase.sh
index 04f6e44..b89f960 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -448,7 +448,7 @@ then
 		then
 			. git-parse-remote
 			error_on_missing_default_upstream "rebase" "rebase" \
-				"against" "git rebase $(gettext '<branch>')"
+				"git rebase $(gettext '<branch>')"
 		fi
 
 		test "$fork_point" = auto && fork_point=t
-- 
2.1.4


^ permalink raw reply related

* Re: Feature request: flagging “volatile” branches for integration/development
From: Duy Nguyen @ 2017-02-04 14:01 UTC (permalink / raw)
  To: Herbert, Marc; +Cc: Git Mailing List, Josh Triplett
In-Reply-To: <70ccb8fc-30f2-5b23-a832-9e470787a945@intel.com>

On Wed, Feb 1, 2017 at 12:12 AM, Herbert, Marc <marc.herbert@intel.com> wrote:
> (Thanks to Josh Triplett[*] for contributing to this message)
>
> Hi,
>
> We often work with development/integration branches that regularly
> rebase, in addition to stable branches that do not. Git is used to share
> two different types of branches:
>   1. Pull requests and merged code with final SHA1s
>   2. Work in progress with volatile SHA1s.
>
> We’d like to have a consistent way to distinguish these two types by
> advertising a branch as “volatile”.

I don't think we have branch metadata (besides reflog). The closet one
is probably config variable branch.<name>.description, which can be
picked up by format-patch to create cover letters. We could do
something similar, e.g. new config branch.<name>.volatile. The
commands can learn about it and apply special treatments if wanted.

But that would be local information only. We don't have ways to
transfer branch metadata (and we definitely don't want to just share
.git/config file with everybody). I suppose extending git protocol for
this is not hard (e.g. appending metadata in the "have" lines). The
hard part may be policy (e.g. what if the user does not want a branch
to be treated volatile by various commands even if it receives such
flag from a git server).
-- 
Duy

^ permalink raw reply

* Re: topological index field for commit objects
From: Jakub Narębski @ 2017-02-04 13:43 UTC (permalink / raw)
  To: Jeff King; +Cc: Git Mailing List
In-Reply-To: <20160720130231.GB17469@sigill.intra.peff.net>

On 20 July 2016 at 15:02, Jeff King <peff@peff.net> wrote:
> On Wed, Jul 20, 2016 at 02:07:38AM +0200, Jakub Narębski wrote:
>> W dniu 2016-06-30 o 00:00, Jeff King pisze:
>>> On Wed, Jun 29, 2016 at 11:49:35PM +0200, Jakub Narębski wrote:
>>
>>>> Do Git use EWAH / EWOK bitmaps for reachability analysis, or is it still
>>>> limited to object counting?
>>>
>>> At GitHub we are using them for --contains analysis, along with mass
>>> ahead/behind (e.g., as in https://github.com/gitster/git/branches). My
>>> plan is to send patches upstream, but they need some cleanup first.
>>
>> Ping. have you got time to clean up those patches?
>
> No, I haven't. Don't hold your breath; it's something I hope to work on
> in the next 6 months, not the next 6 weeks.

Ping, Was there any progress on this front? It is now almost 6 months
later...

Could those patches be made available in a "dirty" form?

TIA,
-- 
Jakub Narębski


-- 
Jakub Narebski

^ permalink raw reply

* Re: [PATCH v2 3/4] introduce new format for git stash create
From: Thomas Gummerer @ 2017-02-04 13:18 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Stephan Beyer, Marc Strapetz, Jeff King, Johannes Schindelin,
	Øyvind A . Holm, Jakub Narębski
In-Reply-To: <xmqqtw8guwfm.fsf@gitster.mtv.corp.google.com>

On 01/30, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> >  create_stash () {
> > -	stash_msg="$1"
> > -	untracked="$2"
> > +	stash_msg=
> > +	untracked=
> > +	new_style=
> > ...
> > +	while test $# != 0
> > +	do
> > +		case "$1" in
> > +		-m|--message)
> > +			shift
> > +			stash_msg="$1"
> 
> 	${1?"-m needs an argument"}
> 
> to error check "git stash create -m<Enter>"?
> 
> > +	if test -z "$new_style"
> > +	then
> > +		stash_msg="$*"
> > +	fi
> 
> This breaks external users who do "git stash create" in the old
> fashioned way, I think, but can be easily fixed with something like:
> 
> 		stash_msg=$1 untracked=$2
> 
> If the existing tests did not catch this, I guess there is a
> coverage gap we may want to fill.  Perhaps add a new test to 3903
> that runs "git stash create message untracked" and makes sure it
> still works?

No I don't think this breaks.  It was never possible to add an
untracked argument to git stash create.  The difference is in a part
of this patch that is snipped out in your reply:

@@ -697,7 +739,7 @@ clear)
        ;;
 create)
        shift
-       create_stash "$*" && echo "$w_commit"
+       create_stash "$@" && echo "$w_commit"
        ;;
 store)
        shift

If I understand this piece correctly (I'm not very proficient in
shell, but my testing seems to agree with me), previously we used $*,
which transformed all arguments to git stash create into one argument
in create_stash.  This needed to change to $@, as otherwise we can't
pull the arguments apart for the new calling style.  The two argument
version of create_stash was only used internally in the save_stash
function.

> > diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> > index 0171b824c9..34e9610bb6 100755
> > --- a/t/t3903-stash.sh
> > +++ b/t/t3903-stash.sh
> > @@ -784,4 +784,22 @@ test_expect_success 'push -m shows right message' '
> >  	test_cmp expect actual
> >  '
> >  
> > +test_expect_success 'deprecated version of stash create stores correct message' '
> > +	>foo &&
> > +	git add foo &&
> > +	STASH_ID=$(git stash create "create test message") &&
> > +	echo "On master: create test message" >expect &&
> > +	git show --pretty=%s ${STASH_ID} | head -n1 >actual &&
> > +	test_cmp expect actual
> > +'
> > +
> > +test_expect_success 'new style stash create stores correct message' '
> > +	>foo &&
> > +	git add foo &&
> > +	STASH_ID=$(git stash create -m "create test message new style") &&
> > +	echo "On master: create test message new style" >expect &&
> > +	git show --pretty=%s ${STASH_ID} | head -n1 >actual &&
> > +	test_cmp expect actual
> > +'
> > +
> >  test_done

-- 
Thomas

^ permalink raw reply

* Re: [PATCH v2] reset: add an example of how to split a commit into two
From: Duy Nguyen @ 2017-02-04 12:36 UTC (permalink / raw)
  To: Philip Oakley; +Cc: Jacob Keller, Git Mailing List, Jacob Keller
In-Reply-To: <E363F108ABEF45CB99C2C721A90B5EA2@PhilipOakley>

On Sat, Feb 4, 2017 at 7:16 PM, Philip Oakley <philipoakley@iee.org> wrote:
> From: "Duy Nguyen" <pclouds@gmail.com>
>>
>> On Sat, Feb 4, 2017 at 3:28 AM, Jacob Keller <jacob.e.keller@intel.com>
>> wrote:
>>>
>>> +------------
>>> +$ git reset HEAD^                           <1>
>>
>>
>> It may be a good idea to add -N here, so that 'add -p' can pick up the
>> new files if they are added in HEAD.
>
>
> When looking at the man page for `reset` [1] it implies that `-N` requires
> `--mixed` also to be given. Is that correct?

Yes. But since --mixed is the default mode, "reset -N" equals "reset --mixed -N"

> Or could the man page be clearer?

If someone makes questions, I guess the answer is yes it should be made clearer.
-- 
Duy

^ permalink raw reply

* Re: [PATCH v2 2/4] stash: introduce push verb
From: Thomas Gummerer @ 2017-02-04 12:19 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Stephan Beyer, Marc Strapetz, Jeff King, Johannes Schindelin,
	Øyvind A . Holm, Jakub Narębski
In-Reply-To: <xmqqy3xsux4g.fsf@gitster.mtv.corp.google.com>

On 01/30, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> > Introduce a new git stash push verb in addition to git stash save.  The
> > push verb is used to transition from the current command line arguments
> > to a more conventional way, in which the message is specified after a -m
> > parameter instead of being a positional argument.
> 
> I think the canonical way to express that is "... the message is
> given as an argument to the -m option" (i.e. some options take an
> argument, some others do not, and the "-m" takes one).
> 
> > This allows introducing a new filename argument to stash single files.
> 
> I do not want them to be "a filename argument", and I do not think
> you meant them as such, either.  
> 
>     This allows us to have pathspecs at the end of the command line
>     arguments like other Git commands do, so that the user can say
>     which subset of paths to stash (and leave others behind).

Yeah, this is much better, thanks.

> > +save_stash () {
> > +	push_options=
> > +	while test $# != 0
> > +	do
> > +		case "$1" in
> > +		-k|--keep-index)
> > +...
> > +		esac
> > +		shift
> > +	done
> 
> It is a bit unfortunate that we need to duplicate the above
> case/esac here.  I do not know if doing it this way:
> 
> 	case "$1" in
> 	--)
> 		shift
> 		break 
> 		;;
> 	--help)
> 		show_help
> 		;;
> 	-*)
> 		# pass all options through to push_stash
> 		push_options="$push_options $1"
> 		;;
> 	*)
> 		break
>                 ;;
> 	esac
> 
> and letting push_stash complain for an unknown option is easier to
> maintain.

I think this will work out nicely.  Will try to implement it this way.

> You are reversing the order of the options in the loop.  Don't.

-- 
Thomas

^ permalink raw reply

* Re: [PATCH v2] reset: add an example of how to split a commit into two
From: Philip Oakley @ 2017-02-04 12:16 UTC (permalink / raw)
  To: Duy Nguyen, Jacob Keller; +Cc: Git Mailing List, Jacob Keller
In-Reply-To: <CACsJy8B2gyw7RQBh6Qfm5HxOyKWted-0ZeDfd2_U3MvWCO1HEA@mail.gmail.com>

From: "Duy Nguyen" <pclouds@gmail.com>
> On Sat, Feb 4, 2017 at 3:28 AM, Jacob Keller <jacob.e.keller@intel.com> 
> wrote:
>> +------------
>> +$ git reset HEAD^                           <1>
>
> It may be a good idea to add -N here, so that 'add -p' can pick up the
> new files if they are added in HEAD.

When looking at the man page for `reset` [1] it implies that `-N` requires 
`--mixed` also to be given. Is that correct? Or could the man page be 
clearer?

>
>> +$ git add -p                                <2>
> -- 
> Duy
>
Philip
[1] https://git-scm.com/docs/git-reset 


^ permalink raw reply

* Re: [PATCH 1/2] doc: add doc for git-push --recurse-submodules=only
From: Cornelius Weig @ 2017-02-04 12:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, bmwill, sbeller
In-Reply-To: <xmqqinotmrhe.fsf@gitster.mtv.corp.google.com>

Shouldn't this be part of v2.12-rc0? I just checked but it's not there.

Cheers,
  Cornelius

^ permalink raw reply

* Re: [PATCH 00/11] nd/worktree-move update
From: Duy Nguyen @ 2017-02-04 11:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <xmqqtw8bf7xn.fsf@gitster.mtv.corp.google.com>

On Sat, Feb 4, 2017 at 1:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Even if you think "the right way" is to add to the iterators, I
> suspect that we can still do incremental fixes?  I agree with the
> order of importance Michael listed in his message (i.e. the index
> and the HEAD first, and then other per-worktree hierarchies at lower
> priority), and I suspect you do, too.  I am not sure that is what
> you called "middle ground", but I think such an incremental approach
> is totally fine.

Yeah index should be fixed independently. Sorry that thought did not
occur to me. I counted HEAD as a ref too (i.e. to be managed/iterated
by refs subsystem) but I guess it's special enough to be dealt with
without ref iterator. I will try to fix these two first.

By middle ground, I was thinking that, now that we have ref iterator
interface, we more or less know what the api should look like for a
ref user like rev-list, so perhaps we could add a custom (ugly)
iterator for per-worktree refs (including reflog). The ugliness is
probably buried in refs/files-backend.c again until we clean it up.
-- 
Duy

^ permalink raw reply

* Re: How to use git show's "%<(<N>[,trunc|ltrunc|mtrunc])"?
From: Duy Nguyen @ 2017-02-04 11:49 UTC (permalink / raw)
  To: Hilco Wijbenga; +Cc: Git Users
In-Reply-To: <CAE1pOi3ThTLSp_0ZJHto4p75Ds5NMGymHrD0ju9axCqA1+NkMA@mail.gmail.com>

On Sat, Feb 4, 2017 at 12:10 AM, Hilco Wijbenga
<hilco.wijbenga@gmail.com> wrote:
>>> How do I get "2017-01-31T17:02:13 | Hilco Wijbenga" to be output?
>>
>> You'll get an ellipsis at the end though.. (i.e. 02:13... | Hilco)
>
> Indeed, that's not very nice. I suppose I can understand the reason
> for it but it mostly defeats the purpose of "trunc", doesn't it?

It depends. When you truncate a commit subject line, you may want an
indicator that you're not seeing the whole line. For fixed fields like
dates when you expect every string to be truncated, then yes there's
not much point to show the ellipsis.
-- 
Duy

^ permalink raw reply

* Re: [PATCH v2] parse-remote: Remove reference to unused op_prep
From: siddharth @ 2017-02-04 11:36 UTC (permalink / raw)
  To: Pranit Bauva; +Cc: Git List
In-Reply-To: <CAFZEwPPyJBD0a+Jkr7EBCurFzvHLvZY9r_SFWS2wyW7QmmP4pg@mail.gmail.com>

On Sat, Feb 04, 2017 at 04:55:45PM +0530, Pranit Bauva wrote:
> Hey SIddharth,
> 
> > Subject: parse-remote: Remove reference to unused op_prep
>                                          ^
> 
> Minor nit: after the colon, we generally don't use the word starting
> with an uppercase letter which I think can be figured out when you run
> `git log -p git-parse-remote.sh`

Oh, I am really sorry to have missed this. I will fix this and send a
third version of this patch.
> 
> On Sat, Feb 4, 2017 at 1:30 PM, Siddharth Kannan
> <kannan.siddharth12@gmail.com> wrote:
> > The error_on_missing_default_upstream helper function learned to
> > take op_prep argument with 15a147e618 ("rebase: use @{upstream}
> > if no upstream specified", 2011-02-09), but as of 045fac5845
> > ("i18n: git-parse-remote.sh: mark strings for translation",
> >  2016-04-19), the argument is no longer used.  Remove it.
> >
> > Signed-off-by: Siddharth Kannan <kannan.siddharth12@gmail.com>
> > ---
> > Thanks a lot, Pranit and Junio for your reviews on the first version of this
> > patch. I have changed the messages accordingly.
> >
> >  contrib/examples/git-pull.sh | 2 +-
> >  git-parse-remote.sh          | 3 +--
> >  git-rebase.sh                | 2 +-
> >  3 files changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/contrib/examples/git-pull.sh b/contrib/examples/git-pull.sh
> > index 6b3a03f..1d51dc3 100755
> > --- a/contrib/examples/git-pull.sh
> > +++ b/contrib/examples/git-pull.sh
> > @@ -267,7 +267,7 @@ error_on_no_merge_candidates () {
> >                 echo "for your current branch, you must specify a branch on the command line."
> >         elif [ -z "$curr_branch" -o -z "$upstream" ]; then
> >                 . git-parse-remote
> > -               error_on_missing_default_upstream "pull" $op_type $op_prep \
> > +               error_on_missing_default_upstream "pull" $op_type \
> >                         "git pull <remote> <branch>"
> >         else
> >                 echo "Your configuration specifies to $op_type $op_prep the ref '${upstream#refs/heads/}'"
> 
> I guess I suggested you to not change the file in contrib/ in my
> earlier email[1] and to which Junio agreed too[2]. Is there any
> confusion?

Oh, you want me to completely remove the contrib/examples/ change
because that's the old shell implementation. Got it, I just checked 
the log for that file and realised that it hasn't been changed 
in a long time.

> 
> > diff --git a/git-parse-remote.sh b/git-parse-remote.sh
> > index d3c3998..9698a05 100644
> > --- a/git-parse-remote.sh
> > +++ b/git-parse-remote.sh
> > @@ -56,8 +56,7 @@ get_remote_merge_branch () {
> >  error_on_missing_default_upstream () {
> >         cmd="$1"
> >         op_type="$2"
> > -       op_prep="$3" # FIXME: op_prep is no longer used
> > -       example="$4"
> > +       example="$3"
> >         branch_name=$(git symbolic-ref -q HEAD)
> >         display_branch_name="${branch_name#refs/heads/}"
> >         # If there's only one remote, use that in the suggestion
> > diff --git a/git-rebase.sh b/git-rebase.sh
> > index 04f6e44..b89f960 100755
> > --- a/git-rebase.sh
> > +++ b/git-rebase.sh
> > @@ -448,7 +448,7 @@ then
> >                 then
> >                         . git-parse-remote
> >                         error_on_missing_default_upstream "rebase" "rebase" \
> > -                               "against" "git rebase $(gettext '<branch>')"
> > +                               "git rebase $(gettext '<branch>')"
> >                 fi
> >
> >                 test "$fork_point" = auto && fork_point=t
> > --
> > 2.1.4
> >
> 
> [1]: http://public-inbox.org/git/CAFZEwPMGTzVuLMSzm8wiNxvia4AV0T79C1ZTfcuO4=Bydz_zQA@mail.gmail.com/
> [2]: http://public-inbox.org/git/xmqqd1ey8rul.fsf@gitster.mtv.corp.google.com/

^ permalink raw reply

* Re: [PATCH v2] parse-remote: Remove reference to unused op_prep
From: Pranit Bauva @ 2017-02-04 11:25 UTC (permalink / raw)
  To: Siddharth Kannan; +Cc: Git List
In-Reply-To: <1486195204-26901-1-git-send-email-kannan.siddharth12@gmail.com>

Hey SIddharth,

> Subject: parse-remote: Remove reference to unused op_prep
                                         ^

Minor nit: after the colon, we generally don't use the word starting
with an uppercase letter which I think can be figured out when you run
`git log -p git-parse-remote.sh`

On Sat, Feb 4, 2017 at 1:30 PM, Siddharth Kannan
<kannan.siddharth12@gmail.com> wrote:
> The error_on_missing_default_upstream helper function learned to
> take op_prep argument with 15a147e618 ("rebase: use @{upstream}
> if no upstream specified", 2011-02-09), but as of 045fac5845
> ("i18n: git-parse-remote.sh: mark strings for translation",
>  2016-04-19), the argument is no longer used.  Remove it.
>
> Signed-off-by: Siddharth Kannan <kannan.siddharth12@gmail.com>
> ---
> Thanks a lot, Pranit and Junio for your reviews on the first version of this
> patch. I have changed the messages accordingly.
>
>  contrib/examples/git-pull.sh | 2 +-
>  git-parse-remote.sh          | 3 +--
>  git-rebase.sh                | 2 +-
>  3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/contrib/examples/git-pull.sh b/contrib/examples/git-pull.sh
> index 6b3a03f..1d51dc3 100755
> --- a/contrib/examples/git-pull.sh
> +++ b/contrib/examples/git-pull.sh
> @@ -267,7 +267,7 @@ error_on_no_merge_candidates () {
>                 echo "for your current branch, you must specify a branch on the command line."
>         elif [ -z "$curr_branch" -o -z "$upstream" ]; then
>                 . git-parse-remote
> -               error_on_missing_default_upstream "pull" $op_type $op_prep \
> +               error_on_missing_default_upstream "pull" $op_type \
>                         "git pull <remote> <branch>"
>         else
>                 echo "Your configuration specifies to $op_type $op_prep the ref '${upstream#refs/heads/}'"

I guess I suggested you to not change the file in contrib/ in my
earlier email[1] and to which Junio agreed too[2]. Is there any
confusion?

> diff --git a/git-parse-remote.sh b/git-parse-remote.sh
> index d3c3998..9698a05 100644
> --- a/git-parse-remote.sh
> +++ b/git-parse-remote.sh
> @@ -56,8 +56,7 @@ get_remote_merge_branch () {
>  error_on_missing_default_upstream () {
>         cmd="$1"
>         op_type="$2"
> -       op_prep="$3" # FIXME: op_prep is no longer used
> -       example="$4"
> +       example="$3"
>         branch_name=$(git symbolic-ref -q HEAD)
>         display_branch_name="${branch_name#refs/heads/}"
>         # If there's only one remote, use that in the suggestion
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 04f6e44..b89f960 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -448,7 +448,7 @@ then
>                 then
>                         . git-parse-remote
>                         error_on_missing_default_upstream "rebase" "rebase" \
> -                               "against" "git rebase $(gettext '<branch>')"
> +                               "git rebase $(gettext '<branch>')"
>                 fi
>
>                 test "$fork_point" = auto && fork_point=t
> --
> 2.1.4
>

[1]: http://public-inbox.org/git/CAFZEwPMGTzVuLMSzm8wiNxvia4AV0T79C1ZTfcuO4=Bydz_zQA@mail.gmail.com/
[2]: http://public-inbox.org/git/xmqqd1ey8rul.fsf@gitster.mtv.corp.google.com/

^ permalink raw reply

* Re: [PATCH v2] reset: add an example of how to split a commit into two
From: Duy Nguyen @ 2017-02-04 11:06 UTC (permalink / raw)
  To: Jacob Keller; +Cc: Git Mailing List, Jacob Keller
In-Reply-To: <20170203202833.17666-1-jacob.e.keller@intel.com>

On Sat, Feb 4, 2017 at 3:28 AM, Jacob Keller <jacob.e.keller@intel.com> wrote:
> +------------
> +$ git reset HEAD^                           <1>

It may be a good idea to add -N here, so that 'add -p' can pick up the
new files if they are added in HEAD.

> +$ git add -p                                <2>
-- 
Duy

^ permalink raw reply

* Re: [PATCH] commit: Optimize number of lstat() calls
From: Johannes Schindelin @ 2017-02-04 10:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Gumbel, Matthew K, git@vger.kernel.org
In-Reply-To: <xmqqd1ey5shc.fsf@gitster.mtv.corp.google.com>

Hi Junio,


On Fri, 3 Feb 2017, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > I could imagine that there is a third option we should consider, too: only
> > lstat() and update the paths that match the pathspec(s) provided on the
> > command line (this is the semantic meaning of the --only option, after
> > all: "I am only interested in these paths as far as this commit is
> > concerned"). What do you think?
> 
> I wondered that myself when I read the first message from Matthew
> and noticed that we always refresh the entire index.  But if it is
> OK to leave the entire index un-refreshed, that would even be
> simpler ;-)

Hah! You're right, that would be much simpler ;-)

Ciao,
Johannes

^ permalink raw reply

* [PATCH v2] parse-remote: Remove reference to unused op_prep
From: Siddharth Kannan @ 2017-02-04  8:00 UTC (permalink / raw)
  To: git; +Cc: pranit.bauva, Siddharth Kannan

The error_on_missing_default_upstream helper function learned to
take op_prep argument with 15a147e618 ("rebase: use @{upstream}
if no upstream specified", 2011-02-09), but as of 045fac5845
("i18n: git-parse-remote.sh: mark strings for translation",
 2016-04-19), the argument is no longer used.  Remove it.

Signed-off-by: Siddharth Kannan <kannan.siddharth12@gmail.com>
---
Thanks a lot, Pranit and Junio for your reviews on the first version of this
patch. I have changed the messages accordingly.

 contrib/examples/git-pull.sh | 2 +-
 git-parse-remote.sh          | 3 +--
 git-rebase.sh                | 2 +-
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/contrib/examples/git-pull.sh b/contrib/examples/git-pull.sh
index 6b3a03f..1d51dc3 100755
--- a/contrib/examples/git-pull.sh
+++ b/contrib/examples/git-pull.sh
@@ -267,7 +267,7 @@ error_on_no_merge_candidates () {
 		echo "for your current branch, you must specify a branch on the command line."
 	elif [ -z "$curr_branch" -o -z "$upstream" ]; then
 		. git-parse-remote
-		error_on_missing_default_upstream "pull" $op_type $op_prep \
+		error_on_missing_default_upstream "pull" $op_type \
 			"git pull <remote> <branch>"
 	else
 		echo "Your configuration specifies to $op_type $op_prep the ref '${upstream#refs/heads/}'"
diff --git a/git-parse-remote.sh b/git-parse-remote.sh
index d3c3998..9698a05 100644
--- a/git-parse-remote.sh
+++ b/git-parse-remote.sh
@@ -56,8 +56,7 @@ get_remote_merge_branch () {
 error_on_missing_default_upstream () {
 	cmd="$1"
 	op_type="$2"
-	op_prep="$3" # FIXME: op_prep is no longer used
-	example="$4"
+	example="$3"
 	branch_name=$(git symbolic-ref -q HEAD)
 	display_branch_name="${branch_name#refs/heads/}"
 	# If there's only one remote, use that in the suggestion
diff --git a/git-rebase.sh b/git-rebase.sh
index 04f6e44..b89f960 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -448,7 +448,7 @@ then
 		then
 			. git-parse-remote
 			error_on_missing_default_upstream "rebase" "rebase" \
-				"against" "git rebase $(gettext '<branch>')"
+				"git rebase $(gettext '<branch>')"
 		fi
 
 		test "$fork_point" = auto && fork_point=t
-- 
2.1.4


^ permalink raw reply related

* How I review patches
From: Junio C Hamano @ 2017-02-04  7:27 UTC (permalink / raw)
  To: git

Here is a write-up on how I review patches posted on the list, in
the hope that knowing what to expect may help contributors [*0*].

But before going into exactly how I do my reviews, here is a short
list of the goals of doing reviews.  I review a patch (or a set of
patches) to ensure that including it to our codebase will NOT:

 * hurt users of existing versions of Git. [*1*]

 * hurt users of the version of Git the change appears in. [*2*]

 * make life harder for developers [*3*] of Git in the future. [*4*]

The remaining paragraphs of this message are not to be taken as me
telling other reviewers how they must conduct their reviews.  I will
queue, and I have queued, patches reviewed by other reviewers
without doing detailed reviews myself, as long as I trust that the
reviewers share the same goals in their reviews, and as long as I
trust their competence and taste.  How they exactly review may be
different from how I would have reviewed the patches, and that is
perfectly fine.

1. Description of the problem being solved.

When I see a patch (or a set of patches), I first read the proposed
log message, documentation update and new in-code comments.

These are places where the contributor can (and is expected to)
explain the motivation behind the change.  I read them to make sure
that they clearly state what problem is being solved, why a
particular solution was chosen, what exactly that solution is, and
what other solutions were considered but discarded.

Just like "X is broken" in a bug report is not clear enough, "Fix X"
is often not enough in the proposed log message, as it is not clear
which part of what X does is wrong in the contributor's mind, and
why the contributor thinks it is broken.  Saying "X currently does Y
but it should do Z instead to help such and such use case." would
help reviewers (and future developers who will read it) understand
the motivation behind the change.

A new feature or enhancement that is worth adding by default needs
to be explained how that new thing works and why it is there to the
end users, so a lack of documentation update is noticed at this
stage as well.

When I find that the explanation is lacking after reading the cover
letter, proposed log message, and documentation updates, I often ask
the contributor to elaborate, before going into the actual diff.  I
often suggest "perhaps you meant this?", and I end up reading the
actual diff to base my best guess on in order to do so.  This is
also where I notice a tricky code whose "why" is under-explained in
in-code comments [*5*].

To such review comments, I do not want the contributor to just say
"yes, you now understand what I wanted to do with this change".  I
want to see the log message, in-code comments and documentation
updated so that other reviewers and future developers will not have
to ask the same question as I asked again.

2. Design of the solution.

After clarifying the original motivation of the contributor, it
sometimes becomes apparent that the patch aims too low and attempts
to solve too narrow an issue.  I would point out that, within the
context of the patches, they can and should solve a wider range of
problems of the same class [*6*].  Or the patch may hurt users in
use cases that the contributor did not consider, and the solution
may need to be designed to cover these cases [*7*].  This design
review may cause us to iterate until we have a good description of
the problem and design of the solution.


3. "Code" review.

Once we made sure that the motivation is made clear, the scope of
the change is refined, and the design of the solution described
clearly, I dive into the code changes.  What I look for primarily
during this phase is to see what the code does matches the desired
behaviour we established before this phase for correctness.  This is
what many people think of as "code review", and it ranges from
spotting style issues and typoes, finding and fixing stupid
off-by-one errors, to noticing future maintainance issues caused by
using a wrong abstration or a misdesigned API.

During the review of the actual code change, I may discover that
some common corner cases are not handled properly, which I would
point out.  Or the contributor may have thought about tricky corner
cases and handled them correctly in the patches, but did not explain
the "what" and "why" in the log message.  Recording what cases were
considered and decided based on what reasoning in the log message is
important to help future developers and sets the course of evolution
of the codebase, and this may result in updates in the "explanation
of the changes" reviewed early in the cycle.

It is not like I never look at the code until log message and
explanation is perfect; to reduce the number of back-and-forth, I do
comment on the code even before it becomes clear if the design is
sound and clearly described.  But at the conceptual level, because
the motivation guides the design and the design guides the
implementation, I tend to review the patches in this order.

Hope the above helps current and future contributors when they are
preparing and reviewing their patch series before submitting.


[Footnotes]

*0* If disclosing this to contributors turns out to be a good idea,
    we may want to add a polished version of this to somewhere in
    Documentation/ next to SubmittingPatches.

*1* For example, we need to be very careful when changing the
    on-disk or on-wire data.  We as developers may always run the
    bleeding-edge version of Git, but how well do users with older
    version of Git interact with our new shiny toys?  Backward
    compatibility issues can hurt users of existing versions Git
    that do not have the change the patch introduces.

*2* That's called a "bug" in general, but can take different forms
    (i.e. implementation bug, documentation bug, design bug, etc.).
    Maybe the documentation promises to (or it can be misread to
    promise to) do A when the actual code does B instead.  Maybe the
    code does A most of the time but silently does something else in
    a corner case, without documenting it.  Maybe the problem the
    patches wanted to solve have two different ways to solve,
    and the patches chose one way to solve and implemented it
    correctly as it documented the feature, but the approach taken
    may be a way that forces users to use Git in an awkward way or
    encourages them to employ a bad workflow, when there is another
    way that helps users better.

*3* Future users of Git cannot be hurt more than the patches
    themselves hurts them immediately.  Patches may make a design
    mistake that makes it hard or impossible to extend a part of the
    system further, and users can be robbed their opportunity to use
    even better Git in the future---but the "even better Git" does
    not exist yet when the patches are accepted anyway, and their
    user experience at least does not get worse, at least.  The
    developers will find a way to work it around and transition
    existing users to allow such an extension into existence and
    their work may be made harder by such a design mistake, though.

*4* A set of patches may add a helper function that is useful for any
    NUL-terminated string, but may make the helper take a pointer to
    a strbuf because the callers to it in the patches happen to all
    have the string in a strbuf.  That forces future developers who
    want to call the helper to either wrap their string in a strbuf
    or to fix the misdesigned API to the helper function to take a
    simple "char *".

    A set of patches may introduce a new helper function to split
    fields in a string whose format is well known throughout the
    system, for which there already exists a helper function to do
    the splitting.  This doubles the amount of work required when
    the format of the string needs to be extended in the future.

    A set of patches may conflate two semantically distinct sets of
    things and try to parse elements of both set with a single
    helper function, only because the elements in these two sets
    happen to be the same in the version immediately after applying
    the patches. This forces future developers who need to add
    more elements to one set without affecting others to split the
    helper into two.

    All of the above make it harder for developers that need to
    enhance the system in the future, but they will NOT hurt users
    of existing version or the version immediately after the patches
    land.  Such a patch series may happen to work correctly
    at that version, and no amount of tests or field trials will
    reveal these maintainance issues.

*5* In-code comments that do not explain why the code does things in
    a particular way but just translates what it does from C to
    English add to maintenance burden of having to keep it in sync
    with the code, without adding much value to the readers of the
    code.  They instead should explain why the code does what it
    does.

*6* For example, a contributor may originally wants to solve
    something only for tags, but it is not uncommon that the issue
    that exists for tags are shared by other kinds of refs, and it
    may be better to solve it uniformly across tags, branches, etc.

*7* "cover"ing other uses cases does not necessarily have to be done
    perfectly.  In less likely situations, it may be OK to punt and
    say "my code cannot handle it" and die() with a message, and
    that is much much better than not considering these situations
    and doing wrong things.
    

^ permalink raw reply

* Re: [PATCH] commit: Optimize number of lstat() calls
From: Junio C Hamano @ 2017-02-04  7:23 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Gumbel, Matthew K, git@vger.kernel.org
In-Reply-To: <alpine.DEB.2.20.1702040129430.3496@virtualbox>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> Besides, leaving the main index not refreshed would mean the user
>> has to pay the refreshing cost when s/he runs other commands "git
>> diff", "git status", etc. after "git commit" for the first time;
>> so...
>
> I am not sure... when you run `git diff` and `git status`, the index is
> refreshed *anyway*, so with the patch under discussion we would save one
> round of lstat() calls, for the same effect.

Yeah, you're right.  The only ones that could be affected are
plumbing commands, and scripts that use plumbing are expected to be
written without relying on the "refreshed"-ness of the index they
are given (iow, if they want to rely on, they are expected to refresh
first before using plumbing commands).  So there is no downside of
leaving the index in an unrefreshed state as long as everbody plays
by the rule.

> I could imagine that there is a third option we should consider, too: only
> lstat() and update the paths that match the pathspec(s) provided on the
> command line (this is the semantic meaning of the --only option, after
> all: "I am only interested in these paths as far as this commit is
> concerned"). What do you think?

I wondered that myself when I read the first message from Matthew
and noticed that we always refresh the entire index.  But if it is
OK to leave the entire index un-refreshed, that would even be
simpler ;-)

^ permalink raw reply

* Re: [PATCH] commit: Optimize number of lstat() calls
From: Johannes Schindelin @ 2017-02-04  0:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Gumbel, Matthew K, git@vger.kernel.org
In-Reply-To: <xmqqwpd678lx.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Fri, 3 Feb 2017, Junio C Hamano wrote:

> Aside from whitespace breakage, I think we should never skip the
> refreshing of the real index that is left after "git commit"
> finishes.
> 
> Between these two calls to refresh_cache(), the one that writes out
> a temporary index that contains the contents of HEAD plus the
> contents of the working tree for the specified paths may be fine
> without refreshing, unless somebody else (like the pre-commit hook)
> looks at it.  But the other one refreshes the real index file that
> will be used after "git commit" returns the control.  Users and
> scripts that run "git commit" inside expect that the entries in the
> resulting index are refreshed after "git commit" returns, and I do
> not think of a safe way to optimizing it out; unlike the other one,
> to which we can say "as long as there is no pre-commit hook, nobody
> will look at it after we are done", there does not an easy-to-check
> set of conditions that we can use to decide when it is safe to skip
> refreshing.
> 
> Besides, leaving the main index not refreshed would mean the user
> has to pay the refreshing cost when s/he runs other commands "git
> diff", "git status", etc. after "git commit" for the first time;
> so...

I am not sure... when you run `git diff` and `git status`, the index is
refreshed *anyway*, so with the patch under discussion we would save one
round of lstat() calls, for the same effect.

I could imagine that there is a third option we should consider, too: only
lstat() and update the paths that match the pathspec(s) provided on the
command line (this is the semantic meaning of the --only option, after
all: "I am only interested in these paths as far as this commit is
concerned"). What do you think?

Ciao,
Johannes

^ permalink raw reply

* Re: [PATCH] commit: Optimize number of lstat() calls
From: Junio C Hamano @ 2017-02-04  6:50 UTC (permalink / raw)
  To: Gumbel, Matthew K; +Cc: git@vger.kernel.org
In-Reply-To: <DA0A42D68346B1469147552440A645039A9C9988@ORSMSX115.amr.corp.intel.com>

Aside from whitespace breakage, I think we should never skip the
refreshing of the real index that is left after "git commit"
finishes.

Between these two calls to refresh_cache(), the one that writes out
a temporary index that contains the contents of HEAD plus the
contents of the working tree for the specified paths may be fine
without refreshing, unless somebody else (like the pre-commit hook)
looks at it.  But the other one refreshes the real index file that
will be used after "git commit" returns the control.  Users and
scripts that run "git commit" inside expect that the entries in the
resulting index are refreshed after "git commit" returns, and I do
not think of a safe way to optimizing it out; unlike the other one,
to which we can say "as long as there is no pre-commit hook, nobody
will look at it after we are done", there does not an easy-to-check
set of conditions that we can use to decide when it is safe to skip
refreshing.

Besides, leaving the main index not refreshed would mean the user
has to pay the refreshing cost when s/he runs other commands "git
diff", "git status", etc. after "git commit" for the first time;
so...


^ permalink raw reply

* [PATCH] Add --gui option to mergetool
From: Denton Liu @ 2017-02-04  6:43 UTC (permalink / raw)
  To: git

* 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.denton@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=<tool>] [-y | --[no-]prompt] [<file>...]
+'git mergetool' [--tool=<tool>] [-g|--gui] [-y | --[no-]prompt] [<file>...]
 
 DESCRIPTION
 -----------
@@ -64,6 +64,12 @@ variable `mergetool.<tool>.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<orderfile>] [file to merge] ...'
+USAGE='[--tool=tool] [-g|--gui] [--tool-help] [-y|--no-prompt|--prompt] [-O<orderfile>] [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


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox