* 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
* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Sverre Rabbelier @ 2009-10-05 21:03 UTC (permalink / raw)
To: Jay Soffian; +Cc: git
In-Reply-To: <1254775583-49452-1-git-send-email-jaysoffian@gmail.com>
Heya,
On Mon, Oct 5, 2009 at 22:46, Jay Soffian <jaysoffian@gmail.com> wrote:
> To create a local branch from the same named remote branch, use
> git checkout -b next origin/next
Since Dscho added the most useful "-t" option to git checkout, why not
suggest that?
$ git checkout -t origin/next # instant win
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Jay Soffian @ 2009-10-05 20:46 UTC (permalink / raw)
To: git; +Cc: Jay Soffian
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
Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
builtin-checkout.c | 43 +++++++++++++++++++++++++++++++++++++++++--
1 files changed, 41 insertions(+), 2 deletions(-)
I dunno, this seems like a lot of code just to make a suggestion to the
user. Is it worth it?
Also, I initially was going to use for_each_remote_ref and compare every
remote ref name to see if it tail matched what the user gave us, but it was
easier to use for_each_remote and build up the remote ref name and then check
for its existence. Not sure if either approach is preferable.
Thoughts/comments?
diff --git a/builtin-checkout.c b/builtin-checkout.c
index d050c37..7f2e215 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -145,6 +145,38 @@ static void fill_mm(const unsigned char *sha1, mmfile_t *mm)
mm->size = size;
}
+struct suggest_new_branch_name_data {
+ const char *name, *found;
+ int matches;
+};
+
+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;
+}
+
+static void suggest_new_branch_name(const char *name)
+{
+ struct suggest_new_branch_name_data data;
+ data.name = name;
+ data.found = NULL;
+ data.matches = 0;
+ for_each_remote(suggest_new_branch_name_compare, &data);
+ if (data.matches == 1)
+ fprintf(stderr, "To create a local branch from the same named remote branch, use\n git checkout -b %s %s\n", name, prettify_refname(data.found));
+}
+
static int checkout_merged(int pos, struct checkout *state)
{
struct cache_entry *ce = active_cache[pos];
@@ -231,8 +263,13 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, ps_matched);
}
- if (report_path_error(ps_matched, pathspec, 0))
+ if (report_path_error(ps_matched, pathspec, 0)) {
+ for (pos = 0; pathspec[pos]; pos++)
+ ;
+ if (pos == 1)
+ suggest_new_branch_name(pathspec[0]);
return 1;
+ }
/* Any unmerged paths? */
for (pos = 0; pos < active_nr; pos++) {
@@ -675,8 +712,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
arg = "@{-1}";
if (get_sha1(arg, rev)) {
- if (has_dash_dash) /* case (1) */
+ if (has_dash_dash) { /* case (1) */
+ suggest_new_branch_name(arg);
die("invalid reference: %s", arg);
+ }
goto no_reference; /* case (3 -> 2) */
}
--
1.6.4.2
^ permalink raw reply related
* Re: [PATCH] Teach 'rebase -i' the command "amend"
From: Johannes Schindelin @ 2009-10-05 20:50 UTC (permalink / raw)
To: Anders Melchiorsen; +Cc: Björn Gustavsson, Sverre Rabbelier, git, gitster
In-Reply-To: <87ab05r5hg.fsf@dylle.kalibalik.dk>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 624 bytes --]
Hi,
On Mon, 5 Oct 2009, Anders Melchiorsen wrote:
> Björn Gustavsson <bgustavsson@gmail.com> writes:
>
> > Thanks for the comments. "reword" it will be then. I'll send a new patch soon.
>
> If you could also make it possible to edit the commit summary line
> right in the git-rebase-todo buffer, that would be great.
>
> 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.
Besides, if you already have typos in the commit subject, you _better_
check the whole commit message, so: double NAK.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Add the --submodule-summary option to the diff option family
From: Johannes Schindelin @ 2009-10-05 20:39 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Junio C Hamano, Lars Hjemli, git
In-Reply-To: <4ACA2DBB.5030908@web.de>
Hi,
On Mon, 5 Oct 2009, Jens Lehmann wrote:
> A bug showed up in the tests, deleted submodules were labelled
> "(not checked out)" instead of "(submodule deleted)". This patch
> fixes that.
Good catch, thanks!
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Teach 'rebase -i' the command "reword"
From: Johannes Schindelin @ 2009-10-05 20:38 UTC (permalink / raw)
To: Björn Gustavsson; +Cc: git, gitster
In-Reply-To: <4ACA1BD1.6050905@gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 256 bytes --]
Hi,
On Mon, 5 Oct 2009, 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.
>
> Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
ACK!
Thanks,
Dscho
^ permalink raw reply
* Re: [PATCH] Teach 'rebase -i' the command "amend"
From: Anders Melchiorsen @ 2009-10-05 19:43 UTC (permalink / raw)
To: Björn Gustavsson; +Cc: Sverre Rabbelier, Johannes Schindelin, git, gitster
In-Reply-To: <6672d0160910050910x3a9aa6a3w742c09e7f2f42187@mail.gmail.com>
Björn Gustavsson <bgustavsson@gmail.com> writes:
> Thanks for the comments. "reword" it will be then. I'll send a new patch soon.
If you could also make it possible to edit the commit summary line
right in the git-rebase-todo buffer, that would be great.
Being in an editor but still not able to fix typos is a nuisance.
Cheers,
Anders.
^ permalink raw reply
* Re: Confusing git pull error message
From: Junio C Hamano @ 2009-10-05 19:36 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Johannes Sixt, John Tapsell, Git List
In-Reply-To: <20091005191257.GA24305@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Oct 05, 2009 at 12:08:38PM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>> > So I think we need something like this. I wasn't able to figure out a
>> > test case to trigger the first code path below, though. It may not be
>> > possible; if we give a refspec on the command line, either it will be a
>> > candidate for merging or, if it does not exist, fetch will barf. So it
>> > may be that we can just collapse it down to a single case.
>>
>> I think you are right.
>
> Nope, I'm not. I figured out one more case that it needs to handle.
> Revised patch coming up in a few minutes.
>
>> By the way, I think the other case arms in the case statement that has the
>> sole caller of this function are never reached, no?
>>
>> Back when you added the check in a74b170 (git-pull: disallow implicit
>> merging to detached HEAD, 2007-01-15), $? referred to the error status of
>> reading HEAD as a symbolic-ref so the check did make sense, but cd67e4d
>> (Teach 'git pull' about --rebase, 2007-11-28) made a stupid mistake that
>> nobody noticed.
>
> Hmm. I'm not sure. I don't see how $? could not be zero, though, because
> the last thing we run is a subshell with sed and tr. But beyond that, we
> actually handle the detached case in error_on_no_merge_candidates
> already. So I think that case statement can simply be collapsed to the
> first case.
>
> -Peff
-- >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.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-pull.sh | 9 +--------
1 files changed, 1 insertions(+), 8 deletions(-)
diff --git a/git-pull.sh b/git-pull.sh
index edf3ce3..66d73eb 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -182,14 +182,7 @@ merge_head=$(sed -e '/ not-for-merge /d' \
case "$merge_head" in
'')
- case $? in
- 0) error_on_no_merge_candidates "$@";;
- 1) echo >&2 "You are not currently on a branch; you must explicitly"
- echo >&2 "specify which branch you wish to merge:"
- echo >&2 " git pull <remote> <branch>"
- exit 1;;
- *) exit $?;;
- esac
+ error_on_no_merge_candidates "$@"
;;
?*' '?*)
if test -z "$orig_head"
--
1.6.5.rc2.77.g2ea4a
^ permalink raw reply related
* Re: Confusing git pull error message
From: Jeff King @ 2009-10-05 19:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, John Tapsell, Git List
In-Reply-To: <20091005191257.GA24305@coredump.intra.peff.net>
On Mon, Oct 05, 2009 at 03:12:57PM -0400, Jeff King wrote:
> > I think you are right.
>
> Nope, I'm not. I figured out one more case that it needs to handle.
> Revised patch coming up in a few minutes.
OK, here it is, which I think covers all of the cases. I also re-wrapped
the text, as I agree with JSixt that it was pretty ugly. I also
re-wrapped some of the existing text, as it gave the very choppy:
Your configuration specifies to merge the ref
'foo' from the remote, but no such ref
was fetched.
It would be really nice to just pipe it through 'fmt', but I suspect
that will create portability problems.
-- >8 --
Subject: [PATCH] pull: improve advice for unconfigured error case
There are several reasons a git-pull invocation might not
have anything marked for merge:
1. We're not on a branch, so there is no branch
configuration.
2. We're on a branch, but there is no configuration for
this branch.
3. We fetched from the configured remote, but the
configured branch to merge didn't get fetched (either
it doesn't exist, or wasn't part of the fetch refspec).
4. We fetched from the non-default remote, but didn't
specify a branch to merge. We can't use the configured
one because it applies to the default remote.
5. We fetched from a specified remote, and a refspec was
given, but it ended up not fetching anything (this is
actually hard to do; if the refspec points to a remote
branch and it doesn't exist, then fetch will fail and
we never make it to this code path. But if you provide
a wildcard refspec like
refs/bogus/*:refs/remotes/origin/*
then you can see this failure).
We have handled (1) and (2) for some time. Recently, commit
a6dbf88 added code to handle case (3).
This patch handles cases (4) and (5), which previously just
fell under other cases, producing a confusing message.
While we're at it, let's rewrap the text for case (3), which
looks terribly ugly as it is.
Signed-off-by: Jeff King <peff@peff.net>
---
git-pull.sh | 16 ++++++++++++----
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/git-pull.sh b/git-pull.sh
index edf3ce3..8b0b6f6 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -90,8 +90,17 @@ error_on_no_merge_candidates () {
curr_branch=${curr_branch#refs/heads/}
upstream=$(git config "branch.$curr_branch.merge")
+ remote=$(git config "branch.$curr_branch.remote")
- if [ -z "$curr_branch" ]; then
+ if [ $# -gt 1 ]; then
+ echo "There are no candidates for merging in the refs that you just fetched."
+ echo "Generally this means that you provided a wildcard refspec which had no"
+ echo "matches on the remote end."
+ elif [ $# -gt 0 ] && [ "$1" != "$remote" ]; then
+ echo "You asked to pull from the remote '$1', but did not specify"
+ echo "a branch to merge. Because this is not the default configured remote"
+ echo "for your current branch, you must specify a branch on the command line."
+ elif [ -z "$curr_branch" ]; then
echo "You are not currently on a branch, so I cannot use any"
echo "'branch.<branchname>.merge' in your configuration file."
echo "Please specify which branch you want to merge on the command"
@@ -116,9 +125,8 @@ error_on_no_merge_candidates () {
echo
echo "See git-config(1) for details."
else
- echo "Your configuration specifies to merge the ref"
- echo "'${upstream#refs/heads/}' from the remote, but no such ref"
- echo "was fetched."
+ echo "Your configuration specifies to merge the ref '${upstream#refs/heads/}' from the"
+ echo "remote, but no such ref was fetched."
fi
exit 1
}
--
1.6.5.rc2.219.g00ca
^ permalink raw reply related
* Re: [PATCH] Speedup bash completion loading
From: Ted Pavlic @ 2009-10-05 19:18 UTC (permalink / raw)
To: Kirill Smelkov; +Cc: Shawn O. Pearce, git
In-Reply-To: <20091005165802.GA24402@tugrik.mns.mnsspb.ru>
> The other possibility is to pregenerate all these lists at git build time, but
> are we going to do it for things under contrib/ ?
IIRC, that's what StGIT does with its completion script. As a
consequence, StGIT completion is substantially faster than git
completion.
--Ted
--
Ted Pavlic <ted@tedpavlic.com>
^ permalink raw reply
* Re: Confusing git pull error message
From: Jeff King @ 2009-10-05 19:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, John Tapsell, Git List
In-Reply-To: <7vljjpacax.fsf@alter.siamese.dyndns.org>
On Mon, Oct 05, 2009 at 12:08:38PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > So I think we need something like this. I wasn't able to figure out a
> > test case to trigger the first code path below, though. It may not be
> > possible; if we give a refspec on the command line, either it will be a
> > candidate for merging or, if it does not exist, fetch will barf. So it
> > may be that we can just collapse it down to a single case.
>
> I think you are right.
Nope, I'm not. I figured out one more case that it needs to handle.
Revised patch coming up in a few minutes.
> By the way, I think the other case arms in the case statement that has the
> sole caller of this function are never reached, no?
>
> Back when you added the check in a74b170 (git-pull: disallow implicit
> merging to detached HEAD, 2007-01-15), $? referred to the error status of
> reading HEAD as a symbolic-ref so the check did make sense, but cd67e4d
> (Teach 'git pull' about --rebase, 2007-11-28) made a stupid mistake that
> nobody noticed.
Hmm. I'm not sure. I don't see how $? could not be zero, though, because
the last thing we run is a subshell with sed and tr. But beyond that, we
actually handle the detached case in error_on_no_merge_candidates
already. So I think that case statement can simply be collapsed to the
first case.
-Peff
^ permalink raw reply
* Re: Confusing git pull error message
From: Junio C Hamano @ 2009-10-05 19:08 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Sixt, John Tapsell, Git List
In-Reply-To: <20091005115308.GA2122@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> So I think we need something like this. I wasn't able to figure out a
> test case to trigger the first code path below, though. It may not be
> possible; if we give a refspec on the command line, either it will be a
> candidate for merging or, if it does not exist, fetch will barf. So it
> may be that we can just collapse it down to a single case.
I think you are right.
By the way, I think the other case arms in the case statement that has the
sole caller of this function are never reached, no?
Back when you added the check in a74b170 (git-pull: disallow implicit
merging to detached HEAD, 2007-01-15), $? referred to the error status of
reading HEAD as a symbolic-ref so the check did make sense, but cd67e4d
(Teach 'git pull' about --rebase, 2007-11-28) made a stupid mistake that
nobody noticed.
> diff --git a/git-pull.sh b/git-pull.sh
> index edf3ce3..a831db5 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -97,6 +97,18 @@ error_on_no_merge_candidates () {
> echo "Please specify which branch you want to merge on the command"
> echo "line and try again (e.g. 'git pull <repository> <refspec>')."
> echo "See git-pull(1) for details."
> + elif [ -n "$1" ]; then
> + if [ $# -gt 1 ]; then
> + echo "You asked to pull from the remote '$1', but none"
> + echo "of the things you asked to fetch were candidates"
> + echo "for merging."
> + else
> + echo "You asked to pull from the remote '$1', but did"
> + echo "not specify a branch to merge. Because this is"
> + echo "not the default configured remote for your current"
> + echo "branch, you must specify a branch on the command"
> + echo "line."
> + fi
> elif [ -z "$upstream" ]; then
> echo "You asked me to pull without telling me which branch you"
> echo "want to merge with, and 'branch.${curr_branch}.merge' in"
>
^ permalink raw reply
* Re: [PATCH] Add the --submodule-summary option to the diff option family
From: Junio C Hamano @ 2009-10-05 19:08 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Jens Lehmann
In-Reply-To: <alpine.DEB.1.00.0910051027010.4985@pacific.mpi-cbg.de>
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.
>> > + }
>> > +
>> > + strbuf_addf(&sb, "Submodule %s %s..", path,
>> > + find_unique_abbrev(one, DEFAULT_ABBREV));
>> > + if (!fast_backward && !fast_forward)
>> > + strbuf_addch(&sb, '.');
> Our output methods translate ANSI, so the strbufs only hold the ANSI
> sequences.
I'll always trust two Johannes's on Windows matters ;-)
> 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.
> 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?
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.
^ permalink raw reply
* Re: [PATCH] Test for correct behaviour on %B(1) and %B(-1)
From: Junio C Hamano @ 2009-10-05 19:08 UTC (permalink / raw)
To: Johannes Gilger; +Cc: Git Mailing List
In-Reply-To: <1253711564-17876-1-git-send-email-heipei@hackvalue.de>
Johannes Gilger <heipei@hackvalue.de> writes:
> seeing as the %B-patch is in your pu you seem to be almost happy with it.
Being in 'pu' does not mean that much; it just means that I do not want to
lose the patch and nothing more. I'll squash the test in anyway.
We may want to build on this further to cover Dscho's line-wrap but that
is independent of this addition.
Thanks.
^ permalink raw reply
* Re: [PATCH] Reserve a slot for argv[0] in default_arg.
From: Jeff King @ 2009-10-05 18:45 UTC (permalink / raw)
To: Petter Urkedal; +Cc: Junio C Hamano, git, Stephen Boyd
In-Reply-To: <20091005063649.GA25040@eideticdew.org>
On Mon, Oct 05, 2009 at 08:36:49AM +0200, Petter Urkedal wrote:
> On 2009-10-04, Junio C Hamano wrote:
> > It is a command specific aliasing mechanism; not even I use the feature
> > these days, since "alias.*" is much easier to use. But there is no strong
> > need to remove it either; it is not too much hassle to keep it for people
> > who do use it. Perhaps deprecate it and remove it in the long run?
>
> I didn't know about alias.*. Excellent. I'll be using that.
Yeah, showbranch.default really seems pointless now. Especially
confusing is the fact that it doesn't do whitespace-splitting, so you
can't do:
git config showbranch.default "--topo-order branch1 branch2"
but instead have to set multiple config variables.
I think deprecation makes sense, but I am in no hurry to get rid of it.
I mainly just wouldn't want people to think it was a useful thing to
learn. :)
> The code is slightly nicer to, I think, but you can probably drop "+ 20"
> in the grow-case now.
I think it could actually just be switched to use ALLOC_GROW.
-Peff
^ permalink raw reply
* Re: [PATCH] Add the --submodule-summary option to the diff option family
From: Jens Lehmann @ 2009-10-05 17:32 UTC (permalink / raw)
To: Johannes Schindelin, Junio C Hamano, Lars Hjemli; +Cc: git
In-Reply-To: <4AC9D6EB.8090002@web.de>
A bug showed up in the tests, deleted submodules were labelled
"(not checked out)" instead of "(submodule deleted)". This patch
fixes that.
--------------------------8<--------------------
[PATCH] fix output for deleted submodules in git diff --submodule-summary
When a submodule has been deleted, add_submodule_odb() returns false
because the directory of the submodule is gone. So we have to test the
second sha for null before we call add_submodule_odb() to get the correct
output.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
submodule.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/submodule.c b/submodule.c
index 11fce7d..54c8de8 100644
--- a/submodule.c
+++ b/submodule.c
@@ -45,12 +45,12 @@ void show_submodule_summary(FILE *f, const char *path,
static const char *format = " %m %s";
int fast_forward = 0, fast_backward = 0;
- if (add_submodule_odb(path))
+ if (is_null_sha1(two))
+ message = "(submodule deleted)";
+ else if (add_submodule_odb(path))
message = "(not checked out)";
else if (is_null_sha1(one))
message = "(new submodule)";
- else if (is_null_sha1(two))
- message = "(submodule deleted)";
else if (!(left = lookup_commit_reference(one)) ||
!(right = lookup_commit_reference(two)))
message = "(commits not present)";
--
1.6.5.rc2.210.gac56a4.dirty
^ permalink raw reply related
* Re: [PATCH] Speedup bash completion loading
From: Kirill Smelkov @ 2009-10-05 16:58 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20091005152504.GE9261@spearce.org>
On Mon, Oct 05, 2009 at 08:25:04AM -0700, Shawn O. Pearce wrote:
> Kirill Smelkov <kirr@mns.spb.ru> wrote:
> > I've tracked down that the most time is spent warming up merge_strategy,
> > all_command & porcelain_command caches.
>
> Nak.
>
> The problem is, during completion when we modify the value the
> change doesn't persist beyond the current completion invocation.
> Thus there is no value in the cache, so every completion attempt
> which needs the list has to rerun the command to compute it.
Yes, my mistake.
To avoid spawning subshell on $(__git_porcelain_commands) I could come up
with "caching" at the upper level, as e.g.
@@ -2107,7 +2111,10 @@ _git ()
--help
"
;;
- *) __gitcomp "$(__git_porcelain_commands) $(__git_aliases)" ;;
+ *) __gitcomp "
+ ${__git_porcelain_commandlist:=$(__git_porcelain_commands)}
+ $(__git_aliases)
+ " ;;
but that's ugly and not maintainable since e.g. __git_all_commands() is used in
several places.
The other possibility is to pregenerate all these lists at git build time, but
are we going to do it for things under contrib/ ?
This could have easy solution if bash provided analog of $() to call a function
and obtain either its stdout or return value without spawning subshell, but up
to now I could not find such a construct in bash's man.
I have to go, but I'll try to come up with something practical in several days.
Thanks for spotting it,
Kirill
^ permalink raw reply
* [PATCH] git-send-email.perl: Fold long header lines to 78 chars
From: Joe Perches @ 2009-10-05 16:24 UTC (permalink / raw)
To: git; +Cc: Jay Soffian
Some MTAs reject or filter long header lines which can
be generated if the cc list is only a few entries.
Fold long header lines to 78 chars to be more rfc compliant.
Signed-off-by: Joe Perches <joe@perches.com>
diff --git a/git-send-email.perl b/git-send-email.perl
index dd821f7..cb8b48b 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -814,6 +814,41 @@ sub sanitize_address
}
+# Fold header lines to 78 chars if possible for better RFC 2822 compliance
+# Does not terminate last line with newline
+sub fold_header
+{
+ my ($folded_line, $separator, @entries) = @_;
+ my $folded_header = "";
+ my $count = 0;
+ my $trim_sep = $separator;
+
+ $trim_sep =~ s/\s+$//;
+
+ foreach my $entry (@entries) {
+ if ($count == 0) {
+ $folded_line = "$folded_line$entry";
+ } elsif ((length($folded_line) + length($entry)) > 78) {
+ if ($folded_header ne "") {
+ $folded_header = "$folded_header$trim_sep\n";
+ }
+ $folded_header = "$folded_header$folded_line";
+ $folded_line = " $entry";
+ } else {
+ $folded_line = "$folded_line$separator$entry";
+ }
+ $count++;
+ }
+
+ if ($count == 0) {
+ $folded_header = "$folded_line";
+ } else {
+ $folded_header = "$folded_header$trim_sep\n$folded_line";
+ }
+
+ return "$folded_header";
+}
+
# Returns 1 if the message was sent, and 0 otherwise.
# In actuality, the whole program dies when there
# is an error sending a message.
@@ -835,10 +870,10 @@ sub send_message
$gitversion = Git::version();
}
- my $cc = join(", ", unique_email_list(@cc));
+ @cc = unique_email_list(@cc);
my $ccline = "";
- if ($cc ne '') {
- $ccline = "\nCc: $cc";
+ if (@cc gt 0) {
+ $ccline = fold_header("\nCc: ", ", ", @cc);
}
my $sanitized_sender = sanitize_address($sender);
make_message_id() unless defined($message_id);
@@ -976,7 +1011,7 @@ X-Mailer: git-send-email $gitversion
if ($smtp_server !~ m#^/#) {
print "Server: $smtp_server\n";
print "MAIL FROM:<$raw_from>\n";
- print "RCPT TO:".join(',',(map { "<$_>" } @recipients))."\n";
+ print fold_header("RCPT TO:", ",", map { "<$_>" } @recipients)."\n";
} else {
print "Sendmail: $smtp_server ".join(' ',@sendmail_parameters)."\n";
}
^ permalink raw reply related
* [PATCH] Teach 'rebase -i' the command "reword"
From: Björn Gustavsson @ 2009-10-05 16:16 UTC (permalink / raw)
To: git; +Cc: gitster
Make it easier to edit just the commit message for a commit
using 'git rebase -i' by introducing the "reword" command.
Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
---
Here is the re-roll of my previous patch.
The only difference is that "amend" (which would be mis-leading)
has been replaced with "reword".
Documentation/git-rebase.txt | 3 +++
git-rebase--interactive.sh | 9 +++++++++
t/lib-rebase.sh | 6 +++---
t/t3404-rebase-interactive.sh | 14 ++++++++++++++
4 files changed, 29 insertions(+), 3 deletions(-)
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".
+
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
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
@@ -340,6 +340,14 @@ do_next () {
pick_one $sha1 ||
die_with_patch $sha1 "Could not apply $sha1... $rest"
;;
+ reword|r)
+ comment_for_reflog reword
+
+ mark_action_done
+ pick_one $sha1 ||
+ die_with_patch $sha1 "Could not apply $sha1... $rest"
+ output git commit --amend
+ ;;
edit|e)
comment_for_reflog edit
@@ -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
# e, edit = use commit, but stop for amending
# s, squash = use commit, but meld into previous commit
#
diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
index 260a231..62f452c 100644
--- a/t/lib-rebase.sh
+++ b/t/lib-rebase.sh
@@ -9,8 +9,8 @@
#
# "[<lineno1>] [<lineno2>]..."
#
-# If a line number is prefixed with "squash" or "edit", the respective line's
-# command will be replaced with the specified one.
+# If a line number is prefixed with "squash", "edit", or "reword", the
+# respective line's command will be replaced with the specified one.
set_fake_editor () {
echo "#!$SHELL_PATH" >fake-editor.sh
@@ -32,7 +32,7 @@ cat "$1".tmp
action=pick
for line in $FAKE_LINES; do
case $line in
- squash|edit)
+ squash|edit|reword)
action="$line";;
*)
echo sed -n "${line}s/^pick/$action/p"
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 4cae019..3a37793 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -470,4 +470,18 @@ test_expect_success 'avoid unnecessary reset' '
test 123456789 = $MTIME
'
+test_expect_success 'reword' '
+ git checkout -b reword-branch master &&
+ FAKE_LINES="1 2 3 reword 4" FAKE_COMMIT_MESSAGE="E changed" git rebase -i A &&
+ git show HEAD | grep "E changed" &&
+ test $(git rev-parse master) != $(git rev-parse HEAD) &&
+ test $(git rev-parse master^) = $(git rev-parse HEAD^) &&
+ FAKE_LINES="1 2 reword 3 4" FAKE_COMMIT_MESSAGE="D changed" git rebase -i A &&
+ git show HEAD^ | grep "D changed" &&
+ FAKE_LINES="reword 1 2 3 4" FAKE_COMMIT_MESSAGE="B changed" git rebase -i A &&
+ git show HEAD~3 | grep "B changed" &&
+ FAKE_LINES="1 reword 2 3 4" FAKE_COMMIT_MESSAGE="C changed" git rebase -i A &&
+ git show HEAD~2 | grep "C changed"
+'
+
test_done
--
1.6.5.rc2.18.g020de
^ permalink raw reply related
* Re: [PATCH] Teach 'rebase -i' the command "amend"
From: Björn Gustavsson @ 2009-10-05 16:10 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Johannes Schindelin, git, gitster
In-Reply-To: <fabb9a1e0910050239h614118cfw8a5055e4ed966dd1@mail.gmail.com>
On Mon, Oct 5, 2009 at 11:39 AM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> 2009/10/5 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>> My objection was that the "upcoming" (yeah, Sverre, I am imitating Duke
>> Nukem Forever here) "merge" command would clash with "msgedit", which was
>> why I suggested "rephrase" (but would be okay with "reword" Junio
>> mentions).
>
> I also dislike 'msgedit' because it's abbrev-ed, I would prefer
> "reword" for that reason.
>
Thanks for the comments. "reword" it will be then. I'll send a new patch soon.
--
Björn Gustavsson, Erlang/OTP, Ericsson AB
^ permalink raw reply
* Re: [RFCv4 3/5] 2/2: Add Python support library for CVS remote helper
From: Johan Herland @ 2009-10-05 15:35 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Git List, Johannes Schindelin, Daniel Barkalow, David Aguilar,
Junio C Hamano
In-Reply-To: <fabb9a1e0910050750q195bdafv5511915fe5d26c02@mail.gmail.com>
On Monday 05 October 2009, Sverre Rabbelier wrote:
> On Mon, Oct 5, 2009 at 16:41, Johan Herland <johan@herland.net> wrote:
> > - Are you planning to share directory structure only, or some of
> > the Python code as well? From the above structure it seems like you
> > want to make use of e.g. util.py and git.py. I'd be delighted if
> > the code is reusable by other remote helpers.
>
> Yes, the main reason I suggested this was because I want to reuse
> util.py and git.py :). Of course, it'd probably be cleaner to move
> the little CVS-specific code that is in git.py atm out of it.
Sure. Refactor away. :)
> > - Do you plan to put the remote helpers into this structure as
> > well, or keep them separate? (currently the cvs remote helper lives
> > separately in git-remote-cvs.py in the project root directory)
>
> I think it's good where it is right now, and I've also placed
> git-remote-hg.py in the root directory.
Ok.
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH 1/6] Open the pack file and keep a map on it.
From: Shawn O. Pearce @ 2009-10-05 15:27 UTC (permalink / raw)
To: Herv?? Cauwelier; +Cc: git
In-Reply-To: <1254593401-18801-2-git-send-email-herve@itaapy.com>
Herv?? Cauwelier <herve@itaapy.com> wrote:
> diff --git a/src/odb.h b/src/odb.h
> index 2f205b2..0311d78 100644
> --- a/src/odb.h
> +++ b/src/odb.h
> @@ -11,9 +11,10 @@
> * uint32_t *fanout = ... the file data at offset 0 ...
> * ntohl(fanout[0]) < ntohl(fanout[1])
> *
> - * The value chosen here for PACK_TOC is such that the above
> + * The value chosen here for IDX_TOC is such that the above
> * cannot be true for an idx v1 file.
> */
> -#define PACK_TOC 0xff744f63 /* -1tOc */
> +#define IDX_TOC 0xff744f63 /* -1tOc */
> +#define PACK_TOC 0x5041434b /* PACK */
FWIW, I wouldn't call the magic string 'PACK' PACK_TOC. TOC here
meant "table of contents". The magic string '-1tOc' for PACK_TOC
is no accident, its trying to show that this file is a table of
contents file.
I think at the time I meant for PACK_TOC to be the pack-*.idx
header magic string, and PACK_SIG or PACK_HDR to be the magic
string for pack-*.pack.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Speedup bash completion loading
From: Shawn O. Pearce @ 2009-10-05 15:25 UTC (permalink / raw)
To: Kirill Smelkov; +Cc: git
In-Reply-To: <1254737039-10404-1-git-send-email-kirr@mns.spb.ru>
Kirill Smelkov <kirr@mns.spb.ru> wrote:
> I've tracked down that the most time is spent warming up merge_strategy,
> all_command & porcelain_command caches.
Nak.
The problem is, during completion when we modify the value the
change doesn't persist beyond the current completion invocation.
Thus there is no value in the cache, so every completion attempt
which needs the list has to rerun the command to compute it.
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 2c2a0d4..4c09d41 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -340,7 +340,6 @@ __git_merge_strategies ()
> }'
> }
> __git_merge_strategylist=
> -__git_merge_strategylist=$(__git_merge_strategies 2>/dev/null)
>
> __git_complete_file ()
> {
> @@ -505,7 +504,6 @@ __git_all_commands ()
> done
> }
> __git_all_commandlist=
> -__git_all_commandlist="$(__git_all_commands 2>/dev/null)"
>
> __git_porcelain_commands ()
> {
> @@ -596,7 +594,6 @@ __git_porcelain_commands ()
> done
> }
> __git_porcelain_commandlist=
> -__git_porcelain_commandlist="$(__git_porcelain_commands 2>/dev/null)"
>
> __git_aliases ()
> {
> --
> 1.6.5.rc2.17.gdbc1b
>
--
Shawn.
^ permalink raw reply
* Re: Interim maintainer tree
From: Jeff King @ 2009-10-05 15:05 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20091005145615.GA9261@spearce.org>
On Mon, Oct 05, 2009 at 07:56:15AM -0700, Shawn O. Pearce wrote:
> Sorry, I proved to be a very poor interim maintainer this cycle.
> I just didn't have the time to keep up with integration each day,
> and then dropped the ball on Thursday and Friday and failed to pull
> anything from Peff or the list. Peff, thanks for keeping up a bit
> and at least having some tips for Junio to pick up from.
It was an interesting experience this time, having two people doing it.
I think the communication overhead may have overwhelmed any help we were
giving each other. Perhaps if there had been more, larger series, it
might have made sense to assign somebody to track a topic and its
iterations, and then finally sign off on it being ready. But being -rc
period, things seem to have settled down, and most patches were one-off
bugfixes all over the place.
-Peff
^ permalink raw reply
* Re: Interim maintainer tree
From: Shawn O. Pearce @ 2009-10-05 14:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7viqevu1zt.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> > Junio is on vaction for the next week. In his absence Peff and I
> > are trying to keep up with current patches in my fork:
> >
> > git://repo.or.cz/git/spearce.git
> > http://repo.or.cz/r/git/spearce.git
> >
> > Right now the tree matches Junio's last push, I'll try to pick up
> > the patches since then and push later today.
>
> Thanks, both.
>
> I've fetched, but haven't fully examined "log ..spearce/*" nor "log ..peff/*"
> yet.
Sorry, I proved to be a very poor interim maintainer this cycle.
I just didn't have the time to keep up with integration each day,
and then dropped the ball on Thursday and Friday and failed to pull
anything from Peff or the list. Peff, thanks for keeping up a bit
and at least having some tips for Junio to pick up from.
> I noticed that some topics in 'pu' have been rebased (not complaining, but
> just making sure I am not hallucinating).
Yes, some topics in 'pu' got rebased. Wednesday I ran RB
after preparing a new master, and then rebuilt pu around it.
Unfortunately I somehow lost a patch on Nick's rev-list series,
not sure how I managed to do that, but I guess I did, sorry.
Were you no longer in the habit of running RB when you rebuilt pu?
--
Shawn.
^ 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