* 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
* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Jay Soffian @ 2009-10-05 21:57 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Johannes Schindelin, git
In-Reply-To: <fabb9a1e0910051426n7f4f8602l8fad733ac3ba82b3@mail.gmail.com>
On Mon, Oct 5, 2009 at 5:26 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> @jay: you assume that if there is more than one matching remote the
> user is experienced (as they have multiple remotes) enough to know
> what to do?
That and it was just an RFC patch, so I just decided to ignore that
case initially.
j.
^ permalink raw reply
* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Sverre Rabbelier @ 2009-10-05 21:26 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jay Soffian, git
In-Reply-To: <alpine.DEB.1.00.0910052314580.4985@pacific.mpi-cbg.de>
Heya,
On Mon, Oct 5, 2009 at 23:17, 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.
If git was a little more interactive I'd say prompt the user, problem solved?
$ git checkout next
No such branch 'next', do you want to check out a local branch for
'origin/next' instead? [Y/n]
@jay: you assume that if there is more than one matching remote the
user is experienced (as they have multiple remotes) enough to know
what to do?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH] Teach 'rebase -i' the command "amend"
From: Johannes Schindelin @ 2009-10-05 21:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: Anders Melchiorsen, Björn Gustavsson, Sverre Rabbelier, git
In-Reply-To: <7vbpkl8s8x.fsf@alter.siamese.dyndns.org>
Hi,
On Mon, 5 Oct 2009, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> Being in an editor but still not able to fix typos is a nuisance.
> >
> > NAK.
> >
> > Supporting that would be totally out of line with the way rebase -i is
> > supposed to work.
>
> If the rebase insn sheet were richer, and had a way to show the full
> message, like this:
>
> pick 4973aa2 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.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>
> I do not see why we shouldn't allow people to edit any part of the above
> to reword.
>
> I would even understand (but not necessarily agree) if somebody wants to
> give the patch text and let users edit to reapply.
>
> So I do not agree with your "totally out of line" at all.
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.
This paradigm has been true even to back when "rebase -i" was still called
"edit-patch-series", and that is the reason I claimed that it is totally
out of line. Because it is.
> > Besides, if you already have typos in the commit subject, you _better_
> > check the whole commit message, so: double NAK.
>
> That sounds a bit too dogmatic.
>
> But I tend to agree with you that we would be better off not accepting
> such a "retitle" patch, as it strongly encourages single-liner commit log
> messages.
Now, that is at least as dogmatic as what I proposed.
> Oh, there was no patch? Then nevermind...
Sorry, but I changed my mind on this attitude. Earlier, you would find me
valuing arguments only when backed up by code. But since the GitTogether
in Berlin I know of at least one discussion where somebody simply had not
enough time to back up a (submodule-related) argument, the validity of the
argument notwithstanding.
So not always is a "no patch? Nevermind" the correct thing to do.
In this particular case, however, I agree. Just touching up the first
line of commit messages is such a special case, and so removed from what a
"rebase" is about that I would claim that this case would be better
handled by a really trivial shell script that launches an editor and then
drives filter-branch based on what the user said.
Ciao,
Dscho
^ permalink raw reply
* Re: previous references
From: Sverre Rabbelier @ 2009-10-05 21:20 UTC (permalink / raw)
To: Daniele Segato; +Cc: Johan Herland, Octavian Râşniţă, git
In-Reply-To: <1254777067.26111.105.camel@localhost>
Heya,
On Mon, Oct 5, 2009 at 23:11, Daniele Segato <daniele.bilug@gmail.com> wrote:
> fatal: ambiguous argument 'HEAD^2': unknown revision or path not in the
> working tree.
> Use '--' to separate paths from revisions
Really? That's awful, shouldn't we at least say something about HEAD
not having that many parents?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Johannes Schindelin @ 2009-10-05 21:17 UTC (permalink / raw)
To: Jay Soffian; +Cc: git
In-Reply-To: <1254775583-49452-1-git-send-email-jaysoffian@gmail.com>
Hi,
On Mon, 5 Oct 2009, Jay Soffian wrote:
> A user who has just cloned a remote repository and wishes to then work on a
> branch other than master may not realize they first need to create the local
> branch. e.g.:
>
> $ 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.
>
> This commit teaches git to make a suggestion to the user:
>
> $ 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.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Teach 'rebase -i' the command "amend"
From: Sverre Rabbelier @ 2009-10-05 21:13 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Anders Melchiorsen, Björn Gustavsson,
git
In-Reply-To: <7vbpkl8s8x.fsf@alter.siamese.dyndns.org>
Heya,
2009/10/5 Junio C Hamano <gitster@pobox.com>:
> If the rebase insn sheet were richer, and had a way to show the full
> message, like this:
But that's not what rebase -i's insn sheet is about is it? It's not
"rewrite my commits so that they are the way I want them", it's about
"change the order of my commits"/squash some/drop some. The polishing
of the commits itself is done after finishing the insn sheet.
> I do not see why we shouldn't allow people to edit any part of the above
> to reword.
Because it then becomes very hard to do any actual reordering. I'm not
saying it's a bad idea, just that it's a bad idea to do it in 'git
rebase -i', conceptually. Although what you suggest would be useful to
me, I just think it should be a different command, git rewrite perhaps
:P. Definitely not "git rebase -i --rewrite-message" though, becaue it
is not at all about rebasing anymore.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: previous references
From: Daniele Segato @ 2009-10-05 21:11 UTC (permalink / raw)
To: Johan Herland; +Cc: Octavian Râşniţă, git
In-Reply-To: <200910041127.29588.johan@herland.net>
Il giorno dom, 04/10/2009 alle 11.27 +0200, Johan Herland ha scritto:
> On Sunday 04 October 2009, Octavian Râşniţă wrote:
> > Are the following commands specifying the same reference?
> >
> > prompt> git log -1 HEAD^^^ ... log entry ...
> > prompt> git log -1 HEAD^~2 ... log entry ...
> > prompt> git log -1 HEAD~1^^ ... log entry ...
> > prompt> git log -1 HEAD~3 ... log entry ...
>
> Yes
the ~ is used to select the first parent of a commit and their
grand-parents
HEAD~ means the parent of the current head
HEAD~2 means the grand-parent
HEAD~3 the grand-grand-parent..
the ^ is used to select a direct parent of a commit
HEAD^ is the same as HEAD~
HEAD^^ is the same as HEAD~2 (parent of the parent)
HEAD^2 is NOT the same of HEAD~2, it means the "second parent" of HEAD:
this make sense only if HEAD has at least two parents (because it is a
merge commit) if it hasn't you'll get:
fatal: ambiguous argument 'HEAD^2': unknown revision or path not in the
working tree.
Use '--' to separate paths from revisions
you can read the same here: http://progit.org/book/ch6-1.html
regards,
Daniele
^ permalink raw reply
* Re: [PATCH] Teach 'rebase -i' the command "amend"
From: Junio C Hamano @ 2009-10-05 21:07 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Anders Melchiorsen, Björn Gustavsson, Sverre Rabbelier, git,
gitster
In-Reply-To: <alpine.DEB.1.00.0910052248500.4985@pacific.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> Being in an editor but still not able to fix typos is a nuisance.
>
> NAK.
>
> Supporting that would be totally out of line with the way rebase -i is
> supposed to work.
If the rebase insn sheet were richer, and had a way to show the full
message, like this:
pick 4973aa2 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.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
I do not see why we shouldn't allow people to edit any part of the above
to reword.
I would even understand (but not necessarily agree) if somebody wants to
give the patch text and let users edit to reapply.
So I do not agree with your "totally out of line" at all.
> Besides, if you already have typos in the commit subject, you _better_
> check the whole commit message, so: double NAK.
That sounds a bit too dogmatic.
But I tend to agree with you that we would be better off not accepting
such a "retitle" patch, as it strongly encourages single-liner commit log
messages.
Oh, there was no patch? Then nevermind...
^ permalink raw reply
* Re: [PATCH] Add the --submodule-summary option to the diff option family
From: Johannes Schindelin @ 2009-10-05 21:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jens Lehmann
In-Reply-To: <7vr5thacb4.fsf@alter.siamese.dyndns.org>
Hi,
On Mon, 5 Oct 2009, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> > + if (prepare_revision_walk(&rev))
> >> > + message = "(revision walker failed)";
> >>
> >> If prepare_revision_walk() failed for whatever reason, can we trust
> >> fast_forward/fast_backward at this point?
> >
> > No, but it is not used in that case, either, because message is not NULL
> > anymore.
>
> It is used in that case a few lines below to decide if you add the third
> dot. That's why I asked.
Well, fair enough.
The answer is: yes, we can still trust fast_forward/fast_backward, as
there is no question that if the first merge base (which must be the only
merge base by definition, in this case) is either "left" or "right", it is
fast_forward or fast_backward, respectively.
So: no worries.
> > 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.
> > In any case, just to safe-guard against sick minds, I can add a check that
> > says that left, right, and all the merge bases _cannot_ have any flags
> > set, otherwise we output "(you should visit a psychiatrist)" or some such.
>
> I wouldn't suggest adding such a kludge. Being insulting to the user when
> we hit a corner case _we_ cannot handle does not help anybody, does it?
Well, I was a little exasperated when I wrote that that you want to handle
that case.
But of course, I should heed Postel's law, and handle the case. Maybe say
something like "(uses superproject's commits)".
> I see two saner options. Doing this list walking in a subprocess so that
> you wouldn't have to worry about object flags at all in this case would
> certainly be easier; the other option obviously is to have a separate
> object pool ala libgit2, but that would be a much larger change.
The reason why I insist avoiding a subprocess is performance. The same
reason holds for a separate object pool: it would just impede the speed,
AFAICT.
Besides, I vividly remember what happened to a patch I posted to be able
to just clear the current object pool. And I cannot imagine a patch
introducing a second pool to be any less complicated.
If you really want the case I illustrated (that the submodule actually
contains commits that already have been shown in the superproject) to be
handled showing the correct submodule summary (and with --first-parent, I
think you will agree that it is a summary, even if it is embedded in a
diff), I could imagine calling a subprocess (for simplicity reasons) _iff_
left, right, or any of the merge bases has a flag set.
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.
Ciao,
Dscho
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox