Git development
 help / color / mirror / Atom feed
* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Junio C Hamano @ 2009-10-06 11:36 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Thomas Rast, Jeff King, Jay Soffian, git
In-Reply-To: <alpine.DEB.1.00.0910061112570.4985@pacific.mpi-cbg.de>

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

>> 4. Are there any (scripted?) use-cases where git-checkout should fail
>>    because it was given an invalid branch name?
>> 
>> The following gives a hint, though they could of course be fixed and
>> the ^0 case doesn't really count:
>> 
>>   $ git grep 'git checkout .*||' -- "*.sh"
>>   git-bisect.sh:        git checkout "$start_head" -- || exit
>>   git-rebase--interactive.sh:  output git checkout $first_parent 2> /dev/null ||
>>   git-rebase--interactive.sh:  output git checkout "$1" ||
>>   git-rebase.sh:git checkout -q "$onto^0" || die "could not detach HEAD"
>>   t/t2007-checkout-symlink.sh:git checkout -f master || exit
>
> Actually, in said cases (with exception of the test case, which should be 
> fine, however, having no remote branches), I would expect the user to be 
> grateful if the DWIMery would happen.

Did you check the context before making that assertion?

 - The one in git-bisect switches to (or detaches at) what was earlier
   written in BISECT_START, which is either a branch name or a commit
   object name, so the user definitely does not want DWIMery if it could
   check out something else --- I do not think DWIMery hurts as long as
   the user does not delete the original branch while bisecting, though.

 - The first one in "rebase -i" is always fed a commit object name;
   DWIMery is not needed (and it would not hurt).

 - The second one in "rebase -i" is about switching to the branch being
   rebased, and it has an explicit check to see if "$1" is a branch name;
   DWIMery is not needed (and it would not hurt because of the check
   before it).

 - The one in "rebase" proper, as Thomas pointed out, is an explicit
   request to detach, so DWIMery won't happen.

The first three cases that could trigger DWIMery fall into "DWIMery does
not hurt because it happens to be a no-op in the way it is used" category,
not "In this case, the users would actively appreciate DWIMery".  IOW,
this does not look particularly a good argument to support DWIMery to me.

About the second one in "rebase -i", and also the corresponding one in
"rebase", which is:

	test -z "$switch_to" || git checkout "$switch_to"

If the command did DWIM, you would fork a local branch from the remote and
immediately rebase it.  Any good git tutorial teaches not to rebase work
by others, and keeping the result of such a rebase on a local branch goes
directly against it [*1*]; the script needs to be updated to protect
itself from DWIMery if we were to change "checkout" in these cases.


[Footnote]

*1* It is quite useful to temporarily rebase others work, e.g. in order to
compare what got changed in the newer version of series, so I wouldn't
object if the user did

    git checkout origin/topic
    git rebase $(git merge-base origin/topic@{1} origin/topic)
    git show-branch origin/topic@{1} HEAD

but notice that it all happens on detached HEAD, not to be kept.

^ permalink raw reply

* Re: [PATCH] Add the --submodule-summary option to the diff option family
From: Junio C Hamano @ 2009-10-06 11:36 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Johannes Schindelin, git
In-Reply-To: <4ACB22E9.3010001@web.de>

Jens Lehmann <Jens.Lehmann@web.de> writes:

>> But I really, really, really want to avoid a fork() in the common case.  I 
>> do have some users on Windows, and I do have a few submodules in that 
>> project.  Having too many fork() calls there would just give Git a bad 
>> reputation.  And it has enough of that, it does not need more.
>
> Me too thinks performance matters here. We do have a repo at my dayjob
> with more than a handful of submodules and its main target platform is
> windows ... so having that perform nicely is a win for us.

Numbers?

I'd prefer to avoid kludges that favors unsubstantiated performance
argument over correctness.

Thanks.

^ permalink raw reply

* Re: [PATCH] Add the --submodule-summary option to the diff option family
From: Jens Lehmann @ 2009-10-06 10:58 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0910052251190.4985@pacific.mpi-cbg.de>

Johannes Schindelin schrieb:
>>> I have no idea why "submodule --summary" uses --first-parent, but 
>>> personally, I would _hate_ it not to see the merged commits in the 
>>> diff.
>>>
>>> For a summary, you might get away with seeing
>>>
>>> 	> Merge bla
>>> 	> Merge blub
>>> 	> Merge this
>>> 	> Merge that
>>>
>>> but in a diff that does not cut it at all.
>> As long as bla/blub/this/that are descriptive enough, I do not see at all
>> why you think "summary" is Ok and "diff" is not.  If your response were
>> "it is just a matter of taste; to some people (or project) --first-parent
>> is useful and for others it is not", I would understand it, and it would
>> make sense to use (or not use) --first-parent consistently between this
>> codepath and "submodule --summary", though.
> 
> You may be used to git.git's quality of naming the branches you merge.
> 
> Sadly, this is not the common case.

IMHO both arguments are valid, using --first-parent really is a matter of
taste *and* it is dependent on the quality of branch naming whether it is
useful or not.

But when both commands shall produce the same output, i think we have to
use --first-parent as default, no? And maybe we could add another option
to diff which can change that behaviour according to users taste?


> But I really, really, really want to avoid a fork() in the common case.  I 
> do have some users on Windows, and I do have a few submodules in that 
> project.  Having too many fork() calls there would just give Git a bad 
> reputation.  And it has enough of that, it does not need more.

Me too thinks performance matters here. We do have a repo at my dayjob
with more than a handful of submodules and its main target platform is
windows ... so having that perform nicely is a win for us.


Jens

^ permalink raw reply

* [RFC PATCH] bash completion: complete refs for git-grep
From: Thomas Rast @ 2009-10-06 10:08 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce

Attempt ref completion once we have seen a regular expression, to help
the user with entering the <treeish> arguments.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

The use-case for this is actually a bit protracted but came up on IRC
yesterday: pasky asked if there was a simple way to grep through a
certain file in all refs.

Turns out git-grep already has half the required support: when given a
series of refs, it prefixes the matches with the ref, so the output is
already in a useful format.

Sadly it does not appear to support --all, --branches or similar
(which would be material for a separate patch).  But bash completion
can step in here: with M-*, it can expand all possible completions for
the current word onto the command line.

This is still RFC because, as you can see in the code below, I tried
to avoid completing at all while the user still needs to supply a
regex.  Sadly, bash turns the COMPREPLY=() into filename completion
anyway.  Is there a way to prevent this?  Otherwise the regex
complication should probably just go away and we can complete refs
always.


 contrib/completion/git-completion.bash |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6fd7e1d..c8cced6 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1048,6 +1048,24 @@ _git_grep ()
 		return
 		;;
 	esac
+
+	local i c=1 have_regex=""
+	while [ $c -lt $COMP_CWORD ]; do
+		i="${COMP_WORDS[c]}"
+		case "$i" in
+		-e) ;;
+		-e*) have_regex="$c" ; break ;;
+		-*) ;;
+		*) have_regex="$c"; break ;;
+		esac
+		c=$((++c))
+	done
+
+	if [ -n "$have_regex" ]; then
+		__gitcomp "$(__git_refs)"
+		return
+	fi
+
 	COMPREPLY=()
 }
 
-- 
1.6.5.rc2.251.g34f85

^ permalink raw reply related

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Johannes Schindelin @ 2009-10-06 10:04 UTC (permalink / raw)
  To: Mikael Magnusson; +Cc: Matthieu Moy, Jeff King, Jay Soffian, git
In-Reply-To: <237967ef0910060241q671baafav93fe6402a4c510c5@mail.gmail.com>

Hi,

On Tue, 6 Oct 2009, Mikael Magnusson wrote:

> I can imagine this happening:
> % git clone git://git.git git
> % git checkout next
> do you want to checkout origin/next? y
> # a few days later
> % git fetch
> % git checkout next
> [freenode] /join #git
> [#git] i did git checkout next but my files are still the same?

I imagined more something like this:

$ git clone git://git.git git
$ git checkout next
Automatically checking out local branch 'next' tracking 'origin/next'.
Please update it with 'git pull'.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] tests: make all test files executable
From: Jeff King @ 2009-10-06 10:00 UTC (permalink / raw)
  To: Mark Rada; +Cc: Junio C Hamano, git
In-Reply-To: <4ACAA15E.6090403@mailservices.uwaterloo.ca>

On Mon, Oct 05, 2009 at 09:46:06PM -0400, Mark Rada wrote:

> 	No changes, just a resend. This should work; I assume
> 	the problem last time was a human error (me :(), or
> 	something weird that happens with saving e-mail drafts
> 	between	Apple Mail and Thunderbird (they share).

This version looks fine (though I fixed up and applied the old one, so
it is only useful as an experment). Whatever you did differently worked.
:)

> 	Jeff, please explain what you meant by `inscrutable
> 	binary'? It is an ASCII text file according to file.
> 	¯\(°_o)/¯

When I looked at in mutt, it was full of binary garbage. But looking at
it more closely, the attachment is bogus. Look at:

  http://article.gmane.org/gmane.comp.version-control.git/129522/raw

You have a message/rfc822 attachment which claims to be encoded using
base64. But there's a bunch of extra text at the top before the base64
starts, which throws off the decoding, leading to the binary garbage.

So the .eml format appears to be a subset of the headers, followed by
the base64-encoded body. But your mail client, in attaching it, marked
it as base64-encoded, which is just wrong. But in theory that is a
problem in transporting the file to the list. For you to "git am" it
yourself, we'll assume you saw the raw contents.

Even then, it is still a confusing format. Instead of the headers
looking like

  From: whatever

there is a line break, so they appear as

  From:
  whatever

and of course there are no mime headers indicating that body is
base64-encoded. So it is definitely not an rfc822 message, which is what
"git am" is expecting.

-Peff

^ permalink raw reply

* Re: [PATCH] Documentation - pt-BR.
From: Miklos Vajna @ 2009-10-06  9:47 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Thiago Farina, git
In-Reply-To: <20091005100910.GA866@coredump.intra.peff.net>

[-- Attachment #1: Type: text/plain, Size: 1015 bytes --]

On Mon, Oct 05, 2009 at 06:09:10AM -0400, Jeff King <peff@peff.net> wrote:
> > Has anybody actually tried to format this document, either before or after
> > your patch?
> 
> No, I didn't, and I should have when I picked up the patch in the first
> place. You are right, asciidoc barfs (both for html and xml generation):
> 
>   ERROR: gittutorial.txt: line 5: first section must be named NAME
>   ERROR: gittutorial.txt: line 9: second section must be named SYNOPSIS

Ah, there is no language config for pt_BR.

$ ls -1 /etc/asciidoc/lang-*
/etc/asciidoc/lang-de.conf
/etc/asciidoc/lang-en.conf
/etc/asciidoc/lang-es.conf
/etc/asciidoc/lang-fr.conf
/etc/asciidoc/lang-hu.conf
/etc/asciidoc/lang-ru.conf

Once asciidoc will have a lang-pt_BR.conf, we could just use -a
lang=pt_BR and it would happily accept this input.

Thiago, could you make a lang-pt_BR.conf? It's less than 100 lines, so
it should be easy for you. (If you can send it to the asciidoc list
directly, it's even better.)

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when  appropriate to do so
From: Mikael Magnusson @ 2009-10-06  9:41 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Johannes Schindelin, Jeff King, Jay Soffian, git
In-Reply-To: <vpqiqesna6x.fsf@bauges.imag.fr>

2009/10/6 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> Hi,
>>
>> On Mon, 5 Oct 2009, Jeff King wrote:
>>
>>> Some devil's advocate questions:
>>>
>>>   1. How do we find "origin/next" given "next"? What are the exact
>>>      lookup rules? Do they cover every case? Do they avoid surprising
>>>      the user?
>>
>> I am sure your strategy would be the same as mine: enumerate all remote
>> branches, strip the remote nickname, and compare.  If there are
>> ambiguities, tell the user and stop.
>>
>>>   2. What do we do if our lookup is ambiguous (e.g., "origin/next" and
>>>      "foobar/next" both exist)?
>>
>> See above.
>
> One problem with this approach is that if users get used to the
> behavior, the command will have great probability to end up in a
> user's script, then the script will "work" as long as there is no
> ambiguity, and cease to work afterwards. And for the user of the
> script, this will sound like "WTF, it was working yesterday and it's
> broken now".
>
> So, the good thing with being strict, even if giving advice in case of
> failure, is that it teaches the user the reliable way to do.

I can imagine this happening:
% git clone git://git.git git
% git checkout next
do you want to checkout origin/next? y
# a few days later
% git fetch
% git checkout next
[freenode] /join #git
[#git] i did git checkout next but my files are still the same?

-- 
Mikael Magnusson

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Matthieu Moy @ 2009-10-06  9:28 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Jeff King, Jay Soffian, git
In-Reply-To: <alpine.DEB.1.00.0910061111410.4985@pacific.mpi-cbg.de>

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

> Hi,
>
> On Mon, 5 Oct 2009, Jeff King wrote:
>
>> Some devil's advocate questions:
>> 
>>   1. How do we find "origin/next" given "next"? What are the exact
>>      lookup rules? Do they cover every case? Do they avoid surprising
>>      the user?
>
> I am sure your strategy would be the same as mine: enumerate all remote 
> branches, strip the remote nickname, and compare.  If there are 
> ambiguities, tell the user and stop.
>
>>   2. What do we do if our lookup is ambiguous (e.g., "origin/next" and
>>      "foobar/next" both exist)?
>
> See above.

One problem with this approach is that if users get used to the
behavior, the command will have great probability to end up in a
user's script, then the script will "work" as long as there is no
ambiguity, and cease to work afterwards. And for the user of the
script, this will sound like "WTF, it was working yesterday and it's
broken now".

So, the good thing with being strict, even if giving advice in case of
failure, is that it teaches the user the reliable way to do.

All that said, I'm not sure how serious this is, but we're in a
"devil's advocate" session, so I'm still allowed to speak ;-).


The other fear I have is to create confusion. Today, it's quite clear
that "next" is not the same as "origin/next". With some DWIMery on top
of this, a naive user may think they are more or less the same, and
then not understand what "git fetch" does and why it's not the same as
"git pull".

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Johannes Schindelin @ 2009-10-06  9:16 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Jeff King, Jay Soffian, git
In-Reply-To: <200910060932.24377.trast@student.ethz.ch>

Hi,

On Tue, 6 Oct 2009, Thomas Rast wrote:

> Jeff King wrote:
> > On Mon, Oct 05, 2009 at 11:17:09PM +0200, Johannes Schindelin wrote:
> > 
> > > > $ git checkout next
> > > > error: pathspec 'next' did not match any file(s) known to git.
> > > 
> > > Actually, we should really think long and hard why we should not 
> > > automatically check out the local branch "next" in that case.  I mean, 
> > > really long and hard, and making sure to take user-friendliness into 
> > > account at least as much as simplicity of implementation.
> > 
> > Some devil's advocate questions:
> > 
> >   1. How do we find "origin/next" given "next"? What are the exact
> >      lookup rules? Do they cover every case? Do they avoid surprising
> >      the user?
> > 
> >   2. What do we do if our lookup is ambiguous (e.g., "origin/next" and
> >      "foobar/next" both exist)?
> > 
> >   3. If our lookup does have ambiguities or corner cases, is it better
> >      to simply be suggesting to the user, rather than proceeding with an
> >      action?
> 
> If I may add another:
> 
> 4. Are there any (scripted?) use-cases where git-checkout should fail
>    because it was given an invalid branch name?
> 
> The following gives a hint, though they could of course be fixed and
> the ^0 case doesn't really count:
> 
>   $ git grep 'git checkout .*||' -- "*.sh"
>   git-bisect.sh:          git checkout "$start_head" -- || exit
>   git-rebase--interactive.sh:                     output git checkout $first_parent 2> /dev/null ||
>   git-rebase--interactive.sh:                     output git checkout "$1" ||
>   git-rebase.sh:git checkout -q "$onto^0" || die "could not detach HEAD"
>   t/t2007-checkout-symlink.sh:git checkout -f master || exit

Actually, in said cases (with exception of the test case, which should be 
fine, however, having no remote branches), I would expect the user to be 
grateful if the DWIMery would happen.

I have to clarify something here: I am not proposing to include a patch 
that does that DWIMery.  We need to discuss the downsides and upsides 
until we can be pretty certain that it does more good than harm.

Unfortunately, this list does not seem to be very inviting to pure users, 
who I hoped would chime in on this issue.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Johannes Schindelin @ 2009-10-06  9:12 UTC (permalink / raw)
  To: Jeff King; +Cc: Jay Soffian, git
In-Reply-To: <20091005225611.GB29335@coredump.intra.peff.net>

Hi,

On Mon, 5 Oct 2009, Jeff King wrote:

> On Mon, Oct 05, 2009 at 11:17:09PM +0200, Johannes Schindelin wrote:
> 
> > > $ git clone git://git.kernel.org/pub/scm/git/git.git
> > > $ cd git
> > > $ git checkout next
> > > error: pathspec 'next' did not match any file(s) known to git.
> > > To create a local branch from the same named remote branch, use
> > >   git checkout -b next origin/next
> > > 
> > > Motivated by http://article.gmane.org/gmane.comp.version-control.git/129528
> > 
> > Actually, we should really think long and hard why we should not 
> > automatically check out the local branch "next" in that case.  I mean, 
> > really long and hard, and making sure to take user-friendliness into 
> > account at least as much as simplicity of implementation.
> 
> Some devil's advocate questions:
> 
>   1. How do we find "origin/next" given "next"? What are the exact
>      lookup rules? Do they cover every case? Do they avoid surprising
>      the user?

I am sure your strategy would be the same as mine: enumerate all remote 
branches, strip the remote nickname, and compare.  If there are 
ambiguities, tell the user and stop.

>   2. What do we do if our lookup is ambiguous (e.g., "origin/next" and
>      "foobar/next" both exist)?

See above.

> 
>   3. If our lookup does have ambiguities or corner cases, is it better
>      to simply be suggesting to the user, rather than proceeding with an
>      action?

See above.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Teach 'rebase -i' the command "reword"
From: Björn Gustavsson @ 2009-10-06  8:25 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: git, gitster
In-Reply-To: <4ACAF515.2060403@gmail.com>

Thanks to both of you!

Stephen Boyd wrote:
> Björn Gustavsson wrote:

> Ok, "stop" being misleading makes sense but I still think the English is
> wrong. Particularly the part about allow editing. Maybe just remove
> "stop", so
> 
>     use commit, but edit the commit message

OK.

> Or we could combine yours, mine, and Hannes' versions.
> 
> The rebase will stop when "pick" has been replaced with "edit" or when a command fails due to merge errors. When you are done editing and/or resolving conflicts you can continue with `git rebase --continue`.
> 

I changed "The rebase" to "'git-rebase' to be consistent with the rest of the documentation.

Here is a new patch, to be applied on top of my original patch.

To avoid confusion, I'll better re-roll the patch. But I'll wait
a day or so, in case there will be more corrections or suggestions
for improvements.

-- >8 --
Subject: [PATCH] Minor polishing of documentation, second attempt

To be applied on top of and combined with my original patch.
---
 Documentation/git-rebase.txt |   10 +++++-----
 git-rebase--interactive.sh   |    2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 52af656..33e0ef1 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -368,17 +368,17 @@ By replacing the command "pick" with the command "edit", you can tell
 the files and/or the commit message, amend the commit, and continue
 rebasing.
 
-If you just want to edit the commit message for a commit, you can replace
-the command "pick" with the command "reword".
+If you just want to edit the commit message for a commit, replace the
+command "pick" with the command "reword".
 
 If you want to fold two or more commits into one, replace the command
 "pick" with "squash" for the second and subsequent commit.  If the
 commits had different authors, it will attribute the squashed commit to
 the author of the first commit.
 
-In both cases, or when a "pick" does not succeed (because of merge
-errors), the loop will stop to let you fix things, and you can continue
-the loop with `git rebase --continue`.
+'git-rebase' will stop when "pick" has been replaced with "edit" or
+when a command fails due to merge errors. When you are done editing
+and/or resolving conflicts you can continue with `git rebase --continue`.
 
 For example, if you want to reorder the last 5 commits, such that what
 was HEAD~4 becomes the new HEAD. To achieve that, you would call
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 30c2f62..a43ee22 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -760,7 +760,7 @@ first and then run 'git rebase --continue' again."
 #
 # Commands:
 #  p, pick = use commit
-#  r, reword = use commit, but allow editing of the commit message
+#  r, reword = use commit, but edit the commit message
 #  e, edit = use commit, but stop for amending
 #  s, squash = use commit, but meld into previous commit
 #
-- 
1.6.5.rc2.18.g020de

^ permalink raw reply related

* Re: [PATCH] Teach 'rebase -i' the command "reword"
From: Stephen Boyd @ 2009-10-06  7:43 UTC (permalink / raw)
  To: Björn Gustavsson; +Cc: git, gitster
In-Reply-To: <4ACAEBBA.9000806@gmail.com>

Björn Gustavsson wrote:
>> How about this?
>>
>> use commit, but stop to edit (or reword?) the commit message
>
> No, I think that would be misleading, as "stop" means exit to the shell
> so that you can run other git commands. (The documentation says:
> "...the loop will stop to let you fix things, and you can continue
> -the loop with `git rebase --continue`")

Ok, "stop" being misleading makes sense but I still think the English is
wrong. Particularly the part about allow editing. Maybe just remove
"stop", so

    use commit, but edit the commit message

>
> -In both cases, or when a "pick" does not succeed (because of merge
> -errors), the loop will stop to let you fix things, and you can continue
> -the loop with `git rebase --continue`.
> +When "pick" has been replaced with "edit" or when a "pick" does not
> +succeed (because of merge errors), the loop will stop to let you fix
> +things, and you can continue the loop with `git rebase --continue`.
>   

Patch looks good, but if you decide to resend due to my comment above
maybe you could think about replacing the word "loop" with "rebase". We
should probably just say "the rebase will stop" and "you can continue
with `git rebase ..."

Or we could combine yours, mine, and Hannes' versions.

The rebase will stop when "pick" has been replaced with "edit" or when a command fails due to merge errors. When you are done editing and/or resolving conflicts you can continue with `git rebase --continue`.

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Thomas Rast @ 2009-10-06  7:32 UTC (permalink / raw)
  To: Jeff King, Johannes Schindelin; +Cc: Jay Soffian, git
In-Reply-To: <20091005225611.GB29335@coredump.intra.peff.net>

Jeff King wrote:
> On Mon, Oct 05, 2009 at 11:17:09PM +0200, Johannes Schindelin wrote:
> 
> > > $ git checkout next
> > > error: pathspec 'next' did not match any file(s) known to git.
> > 
> > Actually, we should really think long and hard why we should not 
> > automatically check out the local branch "next" in that case.  I mean, 
> > really long and hard, and making sure to take user-friendliness into 
> > account at least as much as simplicity of implementation.
> 
> Some devil's advocate questions:
> 
>   1. How do we find "origin/next" given "next"? What are the exact
>      lookup rules? Do they cover every case? Do they avoid surprising
>      the user?
> 
>   2. What do we do if our lookup is ambiguous (e.g., "origin/next" and
>      "foobar/next" both exist)?
> 
>   3. If our lookup does have ambiguities or corner cases, is it better
>      to simply be suggesting to the user, rather than proceeding with an
>      action?

If I may add another:

4. Are there any (scripted?) use-cases where git-checkout should fail
   because it was given an invalid branch name?

The following gives a hint, though they could of course be fixed and
the ^0 case doesn't really count:

  $ git grep 'git checkout .*||' -- "*.sh"
  git-bisect.sh:          git checkout "$start_head" -- || exit
  git-rebase--interactive.sh:                     output git checkout $first_parent 2> /dev/null ||
  git-rebase--interactive.sh:                     output git checkout "$1" ||
  git-rebase.sh:git checkout -q "$onto^0" || die "could not detach HEAD"
  t/t2007-checkout-symlink.sh:git checkout -f master || exit

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: [PATCH] Teach 'rebase -i' the command "reword"
From: Johannes Sixt @ 2009-10-06  7:23 UTC (permalink / raw)
  To: Björn Gustavsson; +Cc: Stephen Boyd, git, gitster
In-Reply-To: <4ACAEBBA.9000806@gmail.com>

Björn Gustavsson schrieb:
> @@ -368,17 +368,17 @@ By replacing the command "pick" with the command "edit", you can tell
>  the files and/or the commit message, amend the commit, and continue
>  rebasing.
>  
> -If you just want to edit the commit message for a commit, you can replace
> -the command "pick" with the command "reword".
> +If you just want to edit the commit message for a commit, replace the
> +command "pick" with the command "reword".
>  
>  If you want to fold two or more commits into one, replace the command
>  "pick" with "squash" for the second and subsequent commit.  If the
>  commits had different authors, it will attribute the squashed commit to
>  the author of the first commit.
>  
> -In both cases, or when a "pick" does not succeed (because of merge
> -errors), the loop will stop to let you fix things, and you can continue
> -the loop with `git rebase --continue`.
> +When "pick" has been replaced with "edit" or when a "pick" does not
> +succeed (because of merge errors), the loop will stop to let you fix
> +things, and you can continue the loop with `git rebase --continue`.

Since "reword" is "pick" + editor, it can fail due to conflicts as well.
Perhaps:

'git-rebase' will stop after an edit command or when a command failed
(due to merge errors). When you are done with your edits or with resolving
merge conflicts, continue with `git rebase --continue`.

(I'm unsure about the mark-up.)

-- Hannes

^ permalink raw reply

* Re: [PATCH] Teach 'rebase -i' the command "reword"
From: Björn Gustavsson @ 2009-10-06  7:03 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: git, gitster
In-Reply-To: <4ACAACAB.3020707@gmail.com>

Stephen Boyd wrote:
> Björn Gustavsson wrote:
>> +If you just want to edit the commit message for a commit, you can replace
>> +the command "pick" with the command "reword".
>> +
> Maybe use the imperative here. So instead of "you can replace" just say
> "replace".

Yes, that's better.
 
> Also, two paragraphs down we say "In both cases ..." but now there are
> three cases right? Maybe we should say

Well spotted. I didn't think about that.

But actually, I think that in "In both cases..." was wrong to begin
with, as there only is one case, namely if the command is "edit".

>> +#  r, reword = use commit, but allow editing of the commit message
> How about this?
> 
> use commit, but stop to edit (or reword?) the commit message

No, I think that would be misleading, as "stop" means exit to the shell
so that you can run other git commands. (The documentation says:
"...the loop will stop to let you fix things, and you can continue
-the loop with `git rebase --continue`")


Here comes a patch that improves the documentation. It should be applied
on top of my previous patch and then combined with it.

If you want me to send a new, combined patch, I will of course do that.

-- >8 --
Subject: [PATCH] Minor polishing of documentation

To be combined with my previous patch.
---
 Documentation/git-rebase.txt |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 52af656..0ae8449 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -368,17 +368,17 @@ By replacing the command "pick" with the command "edit", you can tell
 the files and/or the commit message, amend the commit, and continue
 rebasing.
 
-If you just want to edit the commit message for a commit, you can replace
-the command "pick" with the command "reword".
+If you just want to edit the commit message for a commit, replace the
+command "pick" with the command "reword".
 
 If you want to fold two or more commits into one, replace the command
 "pick" with "squash" for the second and subsequent commit.  If the
 commits had different authors, it will attribute the squashed commit to
 the author of the first commit.
 
-In both cases, or when a "pick" does not succeed (because of merge
-errors), the loop will stop to let you fix things, and you can continue
-the loop with `git rebase --continue`.
+When "pick" has been replaced with "edit" or when a "pick" does not
+succeed (because of merge errors), the loop will stop to let you fix
+things, and you can continue the loop with `git rebase --continue`.
 
 For example, if you want to reorder the last 5 commits, such that what
 was HEAD~4 becomes the new HEAD. To achieve that, you would call
-- 
1.6.5.rc2.18.g020de

^ permalink raw reply related

* Re: [PATCH] Teach 'rebase -i' the command "amend"
From: Johannes Sixt @ 2009-10-06  5:53 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Junio C Hamano, Anders Melchiorsen, Björn Gustavsson,
	Sverre Rabbelier, git
In-Reply-To: <alpine.DEB.1.00.0910052318230.4985@pacific.mpi-cbg.de>

Johannes Schindelin schrieb:
> Are you serious?  "rebase -i" was _always_ about showing an edit script, 
> i.e. to tell Git _what_ you want to do with _which_ commits, identified by 
> short commit names.
> 
> The oneline was _always_ meant as a pure convenience for the user.

Good that you mention it: The one-liner is really convenient. I tend to
replace it by reminders like "====== fix unused var", "===== edit msg",
etc. (after marking the commit "edit") and I'm glad that rebase-i later
prints the one-liner from the insn file rather than the original subject
line before it stops.

(I do it this way because on Windows I can't afford to call rebase-i on a
long patch series for every single task. Rather, I plan the tasks while
the insn editor is open, and this way I keep reminders about what the plan
was.)

No, I definitely don't want the one-liners to end up in the commit
message. ;-)

-- Hannes

^ permalink raw reply

* Re: [PATCH] Teach 'rebase -i' the command "reword"
From: Stephen Boyd @ 2009-10-06  2:34 UTC (permalink / raw)
  To: Björn Gustavsson; +Cc: git, gitster
In-Reply-To: <4ACA1BD1.6050905@gmail.com>

Björn Gustavsson wrote:
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 0aefc34..52af656 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -368,6 +368,9 @@ By replacing the command "pick" with the command "edit", you can tell
>  the files and/or the commit message, amend the commit, and continue
>  rebasing.
>  
> +If you just want to edit the commit message for a commit, you can replace
> +the command "pick" with the command "reword".
> +
Maybe use the imperative here. So instead of "you can replace" just say
"replace".

Also, two paragraphs down we say "In both cases ..." but now there are
three cases right? Maybe we should say

When a "pick" doesn't succeed (because of merge errors) or when "pick"
has been replaced with another command, ...

> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 23ded48..30c2f62 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
>
> @@ -752,6 +760,7 @@ first and then run 'git rebase --continue' again."
>  #
>  # Commands:
>  #  p, pick = use commit
> +#  r, reword = use commit, but allow editing of the commit message
How about this?

use commit, but stop to edit (or reword?) the commit message

^ permalink raw reply

* Re: [PATCH] Teach 'rebase -i' the command "reword"
From: Sebastian Pipping @ 2009-10-06  2:09 UTC (permalink / raw)
  To: git
In-Reply-To: <4ACA1BD1.6050905@gmail.com>

Björn Gustavsson wrote:
> Make it easier to edit just the commit message for a commit
> using 'git rebase -i' by introducing the "reword" command.

Awesome!



Sebastian

^ permalink raw reply

* [PATCH] tests: make all test files executable
From: Mark Rada @ 2009-10-06  1:46 UTC (permalink / raw)
  To: Junio C Hamano, Jeff King; +Cc: git

For consistency with the rest of the test files.

Signed-off-by: Mark Rada <marada@uwaterloo.ca>
---

	No changes, just a resend. This should work; I assume
	the problem last time was a human error (me :(), or
	something weird that happens with saving e-mail drafts
	between	Apple Mail and Thunderbird (they share).

	If this version is also messed up, then I give up.

	Jeff, please explain what you meant by `inscrutable
	binary'? It is an ASCII text file according to file.
	¯\(°_o)/¯

 0 files changed, 0 insertions(+), 0 deletions(-)
 mode change 100644 => 100755 t/t5531-deep-submodule-push.sh
 mode change 100644 => 100755 t/t9501-gitweb-standalone-http-status.sh

diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
old mode 100644
new mode 100755
diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
old mode 100644
new mode 100755
--
1.6.5.rc2

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Jeff King @ 2009-10-05 22:56 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Jay Soffian, git
In-Reply-To: <alpine.DEB.1.00.0910052314580.4985@pacific.mpi-cbg.de>

On Mon, Oct 05, 2009 at 11:17:09PM +0200, Johannes Schindelin wrote:

> > $ git clone git://git.kernel.org/pub/scm/git/git.git
> > $ cd git
> > $ git checkout next
> > error: pathspec 'next' did not match any file(s) known to git.
> > To create a local branch from the same named remote branch, use
> >   git checkout -b next origin/next
> > 
> > Motivated by http://article.gmane.org/gmane.comp.version-control.git/129528
> 
> Actually, we should really think long and hard why we should not 
> automatically check out the local branch "next" in that case.  I mean, 
> really long and hard, and making sure to take user-friendliness into 
> account at least as much as simplicity of implementation.

Some devil's advocate questions:

  1. How do we find "origin/next" given "next"? What are the exact
     lookup rules? Do they cover every case? Do they avoid surprising
     the user?

  2. What do we do if our lookup is ambiguous (e.g., "origin/next" and
     "foobar/next" both exist)?

  3. If our lookup does have ambiguities or corner cases, is it better
     to simply be suggesting to the user, rather than proceeding with an
     action?

-Peff

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Jeff King @ 2009-10-05 22:52 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git
In-Reply-To: <1254775583-49452-1-git-send-email-jaysoffian@gmail.com>

On Mon, Oct 05, 2009 at 04:46:23PM -0400, Jay Soffian wrote:

> +static int suggest_new_branch_name_compare(struct remote *remote, void *priv)
> +{
> +	struct suggest_new_branch_name_data *data = priv;
> +	unsigned char sha1[20];
> +	struct strbuf buf = STRBUF_INIT;
> +	strbuf_addf(&buf, "refs/remotes/%s/%s", remote->name, data->name);
> +	if (resolve_ref(buf.buf, sha1, 1, NULL)) {
> +		data->matches++;
> +		if (data->found)
> +			strbuf_release(&buf);
> +		else
> +			data->found = strbuf_detach(&buf, NULL);
> +	}
> +	return 0;
> +}

This assumes that remote X always has its tracking branches in
refs/remotes/X/*. But that is really dependent on how the fetch refspec
is set up. True, it will be like that for remotes set up by "git remote"
or "git clone", but it isn't universal (and we have tried not to make
that assumption elsewhere, like when finding upstream branches to merge
from).  Doing it right would mean interpreting the refspecs in
remote.*.fetch.

But this is not necessarily about actual remotes, I don't think. It is
really about the names of refs we have, and that you could reference,
but that are not actual tracking branches. It's just that refs/remotes
is the obvious hierarchy there.

But I wonder if what you should do instead is to iterate through each
ref, removing refs/heads/* and refs/tags/* (which are uninteresting, as
they are already part of the normal ref lookup), and then suffix-match.
So looking for "next" would find "refs/remotes/origin/next", or even
"refs/foobar/next" if you had some "foobar" hierarchy.

It would also match "foo" to "refs/remotes/origin/jk/foo". I'm not sure
if that is a feature or a bug, though.


Aside from that, I can't think of anything wrong with the idea.
Personally I find it more chatty than I would want, because I know what
I'm doing. So I would suggest adding an advice.suggestBranchName config
option to voluntarily suppress it.

-Peff

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Johannes Schindelin @ 2009-10-05 22:45 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git
In-Reply-To: <76718490910051500m32878c7dgcc86489933cb2309@mail.gmail.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 598 bytes --]

Hi,

On Mon, 5 Oct 2009, Jay Soffian wrote:

> On Mon, Oct 5, 2009 at 5:17 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Actually, we should really think long and hard why we should not
> > automatically check out the local branch "next" in that case.  I mean,
> > really long and hard, and making sure to take user-friendliness into
> > account at least as much as simplicity of implementation.
> 
> Sure, why not? Are you asking for a patch, or just soliciting 
> conversation?

I am asking for thoughtful arguments for and against my (shyly implied) 
proposal.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when  appropriate to do so
From: Jay Soffian @ 2009-10-05 22:00 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0910052314580.4985@pacific.mpi-cbg.de>

On Mon, Oct 5, 2009 at 5:17 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Actually, we should really think long and hard why we should not
> automatically check out the local branch "next" in that case.  I mean,
> really long and hard, and making sure to take user-friendliness into
> account at least as much as simplicity of implementation.

Sure, why not? Are you asking for a patch, or just soliciting conversation?

j.

^ permalink raw reply

* Re: Confusing git pull error message
From: Jeff King @ 2009-10-05 22:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, John Tapsell, Git List
In-Reply-To: <7vd451ab16.fsf@alter.siamese.dyndns.org>

On Mon, Oct 05, 2009 at 12:36:05PM -0700, Junio C Hamano wrote:

> -- >8 --
> Subject: [PATCH] git-pull: dead code removal
> 
> Back when a74b170 (git-pull: disallow implicit merging to detached HEAD,
> 2007-01-15) added this check, $? referred to the error status of reading
> HEAD as a symbolic-ref; but cd67e4d (Teach 'git pull' about --rebase,
> 2007-11-28) moved the command away from where the check is, and nobody
> noticed the breakage.  Ever since, $? has always been 0 (tr at the end of
> the pipe to find merge_head never fails) and other case arms were never
> reached.
> 
> These days, error_on_no_merge_candidates function is prepared to handle a
> detached HEAD case, which was what the code this patch removes used to
> handle.

Looks good to me.

Acked-by: Jeff King <peff@peff.net>

-Peff

^ permalink raw reply


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