Git development
 help / color / mirror / Atom feed
* Re: [PATCH RESEND] git submodule add: make the <path> parameter optional
From: Johannes Schindelin @ 2009-10-05  9:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jens Lehmann, Lars Hjemli, git
In-Reply-To: <7vtyyek4nz.fsf@alter.siamese.dyndns.org>

Hi,

On Sun, 4 Oct 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > So far, I started submodules by cloning them, doing everything in the 
> > other files needed to integrate, and then actually wondered why "git 
> > submodule add" could not simply take the (relative) path to the 
> > checked-out submodule and deduce the URL from the corresponding config?
> 
> Let me try to rephrase the above to see if I understand what you are
> doing.  When building a top-level superproject that uses two submodules
> from other places, you would do:
> 
> 	$ git init toplevel
>         $ cd toplevel
>         $ git clone $overthere submodule1
>         $ git clone $elsewhere submodule2
>         $ edit Makefile common.h
>         $ git add Makefile common.h submodule1 submodule2
> 
> and by "the corresponding config", you mean "submodule1/.git/config
> already records that it came from $overthere, and there is no reason to
> say it again in 'git submodule add $overthere submodule1'".

Yes, that's what I meant.

But I do like your idea of enhancing "git add" to do this.  Then we can 
keep "git submodule add" as-is, and then it actually makes sense to make 
the <path> parameter optional: "git submodule add" is then about _cloning_ 
and adding the submodule.  Which is not my scenario.

> > So I would actually vote for making the <repository> parameter 
> > optional...
> 
> In your "git submodule add submodule1", it would be quite clear that it is
> a local path and <repository> is being omitted.  On the other hand, if you
> said "git submodule add $overthere" without submodule1, because $overthere
> is not likely to be an in-tree path, it also would be clear that it is
> omitting the path.  IOW, these two typesavers are not mutually exclusive.

I thought about that, but it seems a bit too magical, and it is obvious 
that

	git submodule add $(pwd)/bla

would test that magic.

Ciao,
Dscho

^ permalink raw reply

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

Johannes Schindelin schrieb:
> On Mon, 5 Oct 2009, Johannes Sixt wrote:
>> Junio C Hamano schrieb:
>>> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>>>> +	fwrite(sb.buf, sb.len, 1, f);
>>>> +
>>>> +	if (!message) {
>>>> +		while ((commit = get_revision(&rev))) {
>>>> +			strbuf_setlen(&sb, 0);
>>>> +			if (del)
>>>> +				strbuf_addstr(&sb, commit->object.flags &
>>>> +						SYMMETRIC_LEFT ? del : add);
>>>> +			format_commit_message(commit, format, &sb,
>>>> +					rev.date_mode);
>>>> +			if (del)
>>>> +				strbuf_addstr(&sb, reset);
>>>> +			strbuf_addch(&sb, '\n');
>>>> +			fwrite(sb.buf, sb.len, 1, f);
>> Outch! fwrite doesn't interpret color escapes. AFAICS, this sequence is
>> easy to change such that it uses fprintf().
> 
> Good point.  I changed it to
> 
>                         fprintf(f, "%s", sb.buf);

Thanks. But notice how you are constructing the string in sb from pieces.
I meant to change it to

	fprintf(f, "%s%s%s\n",
			del ? (commit->object.flags & SYMMETRIC_LEFT
					 ? del : add) : "",
			format_commit_message(commit, format, &sb,
					rev.date_mode),
			del ? reset : "");

or similar. We already use this idiom elsewhere.

-- Hannes

^ permalink raw reply

* Re: [PATCH] Add the --submodule-summary option to the diff option  family
From: Johannes Schindelin @ 2009-10-05  9:00 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, git, Jens Lehmann
In-Reply-To: <4AC98FC8.3090202@viscovery.net>

Hi,

On Mon, 5 Oct 2009, Johannes Sixt wrote:

> Junio C Hamano schrieb:
> > Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> >> +	fwrite(sb.buf, sb.len, 1, f);
> >> +
> >> +	if (!message) {
> >> +		while ((commit = get_revision(&rev))) {
> >> +			strbuf_setlen(&sb, 0);
> >> +			if (del)
> >> +				strbuf_addstr(&sb, commit->object.flags &
> >> +						SYMMETRIC_LEFT ? del : add);
> >> +			format_commit_message(commit, format, &sb,
> >> +					rev.date_mode);
> >> +			if (del)
> >> +				strbuf_addstr(&sb, reset);
> > 
> >  - In the "ANSI-terminal only" world view, adding colors to strbuf and
> >    writing it out together with the actual strings is an easy thing to do.
> >    Don't Windows folks have trouble converting this kind of code to their
> >    color control call that is separate from writing strings out?  If it is
> >    not a problem, I do not have any objection to it, but otherwise I'd
> >    suggest not to add any more code that stores color escape sequence in
> >    strbuf, so that we would not make later conversion by Windows folks
> >    harder than necessary.
> 
> Thanks for noticing this! To store color escapes in strbuf is not a
> problem as long as the string is finally written using printf, fprintf, or
> fputs.
> 
> >> +			strbuf_addch(&sb, '\n');
> >> +			fwrite(sb.buf, sb.len, 1, f);
> 
> Outch! fwrite doesn't interpret color escapes. AFAICS, this sequence is
> easy to change such that it uses fprintf().

Good point.  I changed it to

                        fprintf(f, "%s", sb.buf);

BTW we probably need to remove the "TODO: write" from compat/winansi.c...

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Documentation - pt-BR.
From: Miklos Vajna @ 2009-10-05  7:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thiago Farina, git
In-Reply-To: <7vhbuek3ma.fsf@alter.siamese.dyndns.org>

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

On Sun, Oct 04, 2009 at 06:53:49PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> > -NAME
> > +NOME
> >  ----
> >  gittutorial - Um tutorial de introduç??o ao git (para vers??o 1.5.1 ou mais nova)
> >  
> > -SYNOPSIS
> > +SINOPSE
> >  --------
> >  git *
> >  
> > -DESCRIPTION
> > +DESCRIÇ??O
> >  -----------
> 
> Not reading Portuguese, I have two comments.
> 
>  - How well does AsciiDoc and its manpage backend work with these standard
>    section names localized?

As long as the correct -a lang=foo option is passed to asciidoc, the
generated docbook XML will be valid.

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

^ permalink raw reply

* Re: [PATCH] Reserve a slot for argv[0] in default_arg.
From: Petter Urkedal @ 2009-10-05  6:44 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20091004182746.GA22995@coredump.intra.peff.net>

On 2009-10-04, Jeff King wrote:
> t3202 is maybe a bit of weird place to put it, but we don't seem to test
> show-branch anywhere else. It could probably use a "check that
> show-branch works at all" set of tests, but I am not volunteering to
> write such a thing.

Looks good to me.  I'm not so familiar with the source code as I looked
at it first time when I submitted the patch.

> I have always found its output to be one step above
> line noise.

I agree show-branch is nosy, and the actual options I was adding was
"--topo-order master t/*" to show only topic branches.  After rebasing
them against master, show-branch with these arguments gives a nice
overview.  Thanks Junio's tip I now have

    alias.show-topics = show-branch --topo-order master t/*

^ permalink raw reply

* Re: [PATCH] Reserve a slot for argv[0] in default_arg.
From: Petter Urkedal @ 2009-10-05  6:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git, Stephen Boyd
In-Reply-To: <7v4oqen6my.fsf@alter.siamese.dyndns.org>

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.
 
> > Correct. Junio sent a patch to fix this problem in June[1]. I guess he
> > must have dropped his own patch, or he wasn't satisfied with how parse
> > options clobbers things.
> >
> > [1] http://article.gmane.org/gmane.comp.version-control.git/121142
> 
> I had it kept still in my Inbox; thanks for noticing.  Petter's patch does
> essentially the same thing, but the old patch had a better log message
> that described where in the history the fix should apply, so I'd probably
> use that with your test squashed in.

The code is slightly nicer to, I think, but you can probably drop "+ 20"
in the grow-case now.

^ permalink raw reply

* Re: [PATCH] Teach 'rebase -i' the command "amend"
From: Junio C Hamano @ 2009-10-05  6:33 UTC (permalink / raw)
  To: Björn Gustavsson; +Cc: Johannes Schindelin, git, gitster
In-Reply-To: <6672d0160910042308v7280dcadyff97b977bcfe12c3@mail.gmail.com>

Björn Gustavsson <bgustavsson@gmail.com> writes:

> I found this thread from January 2009:
>
> http://thread.gmane.org/gmane.comp.version-control.git/105738
>
> Having read the thread, I agree that "amend" would be misleading.

Yeah, I thought "reword" might be a good command name back then.

Was the naming the only issue in the discussion there?

^ permalink raw reply

* Re: [PATCH] MSVC: fix build warnings
From: Junio C Hamano @ 2009-10-05  6:31 UTC (permalink / raw)
  To: Michael Wookey; +Cc: Junio C Hamano, git
In-Reply-To: <d2e97e800910042238t3ba02d59ud4e7f5b3b2b11183@mail.gmail.com>

Michael Wookey <michaelwookey@gmail.com> writes:

> I still get the warning with the current git.git:master (dbc1b1f7)

Hmph, I somehow thought Shawn already applied the patch I sent
$gmane/129285, but it seems he hasn't.

^ permalink raw reply

* Re: [PATCH 3/3] Add "%w" to pretty formats, which rewraps the commit message
From: Junio C Hamano @ 2009-10-05  6:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Gilger, Git Mailing List, Johannes Schindelin
In-Reply-To: <7v63b9gsg1.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> One issue %w() sidesteps is handing of single liner commit log messages
> (this is not a new issue your %B(n) introduces).  "%s%n%b" will give us
> the original message only when the log has some contents in addition to
> the single-line summary.  Otherwise we will get an extra blank line.
>
> Perhaps we could extend the pretty-printer so that it understands %+x
> notation, which expands to %n%x when %x expands to a non-empty result, and
> otherwise it expands to empty, as a generic extension applicable to any
> format specifier 'x'.  If we have such a notation, "%s%+b", would be a
> reasonable way to say "give us the original commit log message here", and
> we won't need %w(i,j,w) -- we can instead say %S(i,j,w)%+B(i,j,w), or
> %s%+B(i,j,w) depending on what you want.

This teaches the machinery to add a separator LF before any non-empty
expansion of '%x' if you ask '%+x', and also removes LF if '%x' expands to
an empty string if you ask '%-x', for any supported expansion placeholder
'x' it supports.

With the first two patches Dscho posted (and then polished in his tree),
it shouldn't be too hard to update your %B(n) to %B(i,j,w) and also add
%S(i,j,w) in a similar way, to allow people to say %S(i,j,w)%+B(i,j,w)
instead of (or in addition to) his %w(i,j,w).



 pretty.c                   |   42 ++++++++++++++++++++++++++++++++++++++++--
 t/t6006-rev-list-format.sh |   22 ++++++++++++++++++++++
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/pretty.c b/pretty.c
index f5983f8..081feb6 100644
--- a/pretty.c
+++ b/pretty.c
@@ -595,8 +595,8 @@ static void format_decoration(struct strbuf *sb, const struct commit *commit)
 		strbuf_addch(sb, ')');
 }
 
-static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
-                               void *context)
+static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
+				void *context)
 {
 	struct format_commit_context *c = context;
 	const struct commit *commit = c->commit;
@@ -739,6 +739,44 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
 	return 0;	/* unknown placeholder */
 }
 
+static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
+				 void *context)
+{
+	int consumed;
+	size_t orig_len;
+	enum {
+		NO_MAGIC,
+		ADD_LF_BEFORE_NON_EMPTY,
+		DEL_LF_BEFORE_EMPTY,
+	} magic = NO_MAGIC;
+
+	switch (placeholder[0]) {
+	case '-':
+		magic = DEL_LF_BEFORE_EMPTY;
+		break;
+	case '+':
+		magic = ADD_LF_BEFORE_NON_EMPTY;
+		break;
+	default:
+		break;
+	}
+	if (magic != NO_MAGIC)
+		placeholder++;
+
+	orig_len = sb->len;
+	consumed = format_commit_one(sb, placeholder, context);
+	if (magic == NO_MAGIC)
+		return consumed;
+
+	if ((orig_len == sb->len) && magic == DEL_LF_BEFORE_EMPTY) {
+		while (sb->len && sb->buf[sb->len - 1] == '\n')
+			strbuf_setlen(sb, sb->len - 1);
+	} else if ((orig_len != sb->len) && magic == ADD_LF_BEFORE_NON_EMPTY) {
+		strbuf_insert(sb, orig_len, "\n", 1);
+	}
+	return consumed + 1;
+}
+
 void format_commit_message(const struct commit *commit,
 			   const void *format, struct strbuf *sb,
 			   enum date_mode dmode)
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 59d1f62..18a77a7 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -162,4 +162,26 @@ test_expect_success 'empty email' '
 	}
 '
 
+test_expect_success 'del LF before empty (1)' '
+	git show -s --pretty=format:"%s%n%-b%nThanks%n" HEAD^^ >actual &&
+	test $(wc -l <actual) = 2
+'
+
+test_expect_success 'del LF before empty (2)' '
+	git show -s --pretty=format:"%s%n%-b%nThanks%n" HEAD >actual &&
+	test $(wc -l <actual) = 6 &&
+	grep "^$" actual
+'
+
+test_expect_success 'add LF before non-empty (1)' '
+	git show -s --pretty=format:"%s%+b%nThanks%n" HEAD^^ >actual &&
+	test $(wc -l <actual) = 2
+'
+
+test_expect_success 'add LF before non-empty (2)' '
+	git show -s --pretty=format:"%s%+b%nThanks%n" HEAD >actual &&
+	test $(wc -l <actual) = 6 &&
+	grep "^$" actual
+'
+
 test_done
.

^ permalink raw reply related

* Re: [PATCH] Add the --submodule-summary option to the diff option family
From: Johannes Sixt @ 2009-10-05  6:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git, Jens Lehmann
In-Reply-To: <7vbpkmn6oi.fsf@alter.siamese.dyndns.org>

Junio C Hamano schrieb:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>> +	fwrite(sb.buf, sb.len, 1, f);
>> +
>> +	if (!message) {
>> +		while ((commit = get_revision(&rev))) {
>> +			strbuf_setlen(&sb, 0);
>> +			if (del)
>> +				strbuf_addstr(&sb, commit->object.flags &
>> +						SYMMETRIC_LEFT ? del : add);
>> +			format_commit_message(commit, format, &sb,
>> +					rev.date_mode);
>> +			if (del)
>> +				strbuf_addstr(&sb, reset);
> 
>  - In the "ANSI-terminal only" world view, adding colors to strbuf and
>    writing it out together with the actual strings is an easy thing to do.
>    Don't Windows folks have trouble converting this kind of code to their
>    color control call that is separate from writing strings out?  If it is
>    not a problem, I do not have any objection to it, but otherwise I'd
>    suggest not to add any more code that stores color escape sequence in
>    strbuf, so that we would not make later conversion by Windows folks
>    harder than necessary.

Thanks for noticing this! To store color escapes in strbuf is not a
problem as long as the string is finally written using printf, fprintf, or
fputs.

>> +			strbuf_addch(&sb, '\n');
>> +			fwrite(sb.buf, sb.len, 1, f);

Outch! fwrite doesn't interpret color escapes. AFAICS, this sequence is
easy to change such that it uses fprintf().

-- Hannes

^ permalink raw reply

* Re: [PATCH] Teach 'rebase -i' the command "amend"
From: Björn Gustavsson @ 2009-10-05  6:08 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0910042308200.4985@pacific.mpi-cbg.de>

2009/10/4 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> I thought that we had a discussion about this and that the consensus was
> that "amend" would be misleading.  Maybe you can find that thread in
> GMane?

I found this thread from January 2009:

http://thread.gmane.org/gmane.comp.version-control.git/105738

Having read the thread, I agree that "amend" would be misleading.

There were several suggestions for alternate command names
in that thread, for example:

"msg", "msgedit", "message", "reword", "rephrase"

It think that "msgedit" was suggested by several people. ("editmsg"
was also suggested, but it is not possible as the abbreviation "e" would
become ambiguous.)

Would the patch have a chance to be accepted if I renamed
the new command to "msgedit"?

/Björn

-- 
Björn Gustavsson, Erlang/OTP, Ericsson AB

^ permalink raw reply

* Re: [PATCH] MSVC: fix build warnings
From: Michael Wookey @ 2009-10-05  5:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7hvak1ec.fsf@alter.siamese.dyndns.org>

2009/10/5 Junio C Hamano <gitster@pobox.com>:
> Michael Wookey <michaelwookey@gmail.com> writes:
>
>> I can't build with -Werror on Ubuntu 9.04 (gcc 4.3.3) because of the following:
>>
>>   http://article.gmane.org/gmane.comp.version-control.git/127477
>
> I think that has been fixed already while I was away ;-)

I still get the warning with the current git.git:master (dbc1b1f7) and
git.git:next (8ea19b84). Would you consider taking the original patch
to fix the build warning? Perhaps I should resend the patch as a
"partial revert" of eb3a9dd which introduced this warning as described
here:

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

What do you think?

^ permalink raw reply

* Re: "Not currently on any branch"
From: Junio C Hamano @ 2009-10-05  4:19 UTC (permalink / raw)
  To: Timothy Washington; +Cc: git
In-Reply-To: <106990.92203.qm@web111508.mail.gq1.yahoo.com>

Timothy Washington <timothyjwashington@yahoo.ca> writes:

> Thanks for all the responses so far. But if you take a look at my repo
> (http://repo.or.cz/w/Bookkeeping.git), at the bottom of the page,
> there's clearly a 'ui-integration' branch. But if I try to go to my
> ui-integration branch, I get the message below. So I'm just clueless as
> to where it went. If use the -b option, then I'll create a new
> branch. But I don't want that. I want to keep all the data that was in
> my original 'ui-integration' branch.
>
> [timothyw] ~/Projects/Bookkeeping.4 $ git checkout ui-integration
> error: pathspec 'ui-integration' did not match any file(s) known to git.

I do not think it has anything to do with "Not currently on any branch",
but judging from this

> [remote "origin"]
>     url = http://repo.or.cz/r/Bookkeeping.git
>     fetch = +refs/heads/*:refs/remotes/origin/*

one possibility to see the above error message is to do this:

    $ git clone http://repo.or.cz/r/Bookkeeping.git
    $ cd Bookkeeping
    $ git checkout ui-integration
    error: pathspec 'ui-integration' did not match any file(s) known to git.

In a clone, your local branch namespace is not cluttered with all the
different branches your upstream repository has.  To wit:

    $ git branch
    * master
    $ git branch -a
    * master
      remotes/origin/HEAD -> origin/master
      remotes/origin/master
      remotes/origin/ui-integration

If you want to further work on the ui-integration topic, you would do
something like:

    $ git checkout -b ui-integration origin/ui-integration
    Branch ui-integration set up to track remote branch ui-integration from origin.
    Switched to a new branch 'ui-integration'
    $ git branch
      master
    * ui-integration

On the other hand, if you are not interested in working on that topic but
just want to look at it, e.g. merge it to your master:

    $ git branch
    * master
    $ git merge origin/ui-integration

without creating a local ui-integration branch at all (iow, skip that
"checkout -b" step above altogether).

^ permalink raw reply

* Re: "Not currently on any branch"
From: Timothy Washington @ 2009-10-05  4:01 UTC (permalink / raw)
  To: git


Thanks for all the responses so far. But if you take a look at my repo (http://repo.or.cz/w/Bookkeeping.git), at the bottom of the page, there's clearly a 'ui-integration' branch. But if I try to go to my ui-integration branch, I get the message below. So I'm just clueless as to where it went. If use the -b option, then I'll create a new branch. But I don't want that. I want to keep all the data that was in my original 'ui-integration' branch. 

[timothyw] ~/Projects/Bookkeeping.4 $ git checkout ui-integration
error: pathspec 'ui-integration' did not match any file(s) known to git.


These are what's in the files under my .git/ directory. And again, I'm foggy as to where my ui-integration branch went and to how to get it back. 

"HEAD"
cee341c53a249f7d003a12cfeb8cc743275d028f

"ORIG_HEAD"
baca7c9f43e44d67406d3671e0afd84eaea870a3

"config"
[core]
    repositoryformatversion = 0
    filemode = true
    bare = false
    logallrefupdates = true
    ignorecase = true
[remote "origin"]
    url = http://repo.or.cz/r/Bookkeeping.git
    fetch = +refs/heads/*:refs/remotes/origin/*
[branch "master"]
    remote = origin
    merge = refs/heads/master

"description"
Unnamed repository; edit this file to name it for gitweb.

fig. 1 


Thanks
Tim 






________________________________
From: Clemens Buchacher <drizzd@aon.at>
To: Tim <timothyjwashington@yahoo.ca>
Cc: git@vger.kernel.org
Sent: Sunday, October 4, 2009 3:22:29 AM
Subject: Re: "Not currently on any branch"

On Fri, Oct 02, 2009 at 08:08:52PM +0000, Tim wrote:
> I have some code in a git repo that is "Not currently on any branch". Now,
> there's the master branch and another branch 'ui-integration' that I'm
> using in this project. I don't know how the project got into this headless
> state, but I need to be using the 'ui-integration' branch. 

It can happen either by explicitly detaching HEAD using "git checkout
<commit>", or if you used rebase and it is still in progress.

Clemens

________________________________
Instant message from any web browser! Try the new Yahoo! Canada Messenger for the Web BETA


      __________________________________________________________________
Yahoo! Canada Toolbar: Search from anywhere on the web, and bookmark your favourite sites. Download it now
http://ca.toolbar.yahoo.com.

^ permalink raw reply

* Re: [PATCH] MSVC: fix build warnings
From: Junio C Hamano @ 2009-10-05  2:41 UTC (permalink / raw)
  To: Michael Wookey; +Cc: Junio C Hamano, git
In-Reply-To: <d2e97e800910021628t13bba313he119ba59babdecee@mail.gmail.com>

Michael Wookey <michaelwookey@gmail.com> writes:

> I can't build with -Werror on Ubuntu 9.04 (gcc 4.3.3) because of the following:
>
>   http://article.gmane.org/gmane.comp.version-control.git/127477

I think that has been fixed already while I was away ;-)

^ permalink raw reply

* Re: [PATCH 2/6 (v4)] basic revision cache system, no integration or features
From: Junio C Hamano @ 2009-10-05  2:15 UTC (permalink / raw)
  To: Nick Edelen
  Cc: Junio C Hamano, Nicolas Pitre, Johannes Schindelin, Sam Vilain,
	Michael J Gruber, Jeff King, Shawn O. Pearce, Andreas Ericsson,
	Christian Couder, git@vger.kernel.org
In-Reply-To: <op.u061bavktdk399@sirnot.ed.ac.uk>

The entire series is unusable due to whitespace breakage.

^ permalink raw reply

* Re: [PATCH] Documentation - pt-BR.
From: Junio C Hamano @ 2009-10-05  1:53 UTC (permalink / raw)
  To: Thiago Farina; +Cc: git
In-Reply-To: <1253730339-11146-1-git-send-email-tfransosi@gmail.com>

Thiago Farina <tfransosi@gmail.com> writes:

> diff --git a/Documentation/pt_BR/gittutorial.txt b/Documentation/pt_BR/gittutorial.txt
> index 81e7ad7..beba065 100644
> --- a/Documentation/pt_BR/gittutorial.txt
> +++ b/Documentation/pt_BR/gittutorial.txt
> @@ -1,15 +1,15 @@
>  gittutorial(7)
>  ==============
>  
> -NAME
> +NOME
>  ----
>  gittutorial - Um tutorial de introdução ao git (para versão 1.5.1 ou mais nova)
>  
> -SYNOPSIS
> +SINOPSE
>  --------
>  git *
>  
> -DESCRIPTION
> +DESCRIÇÃO
>  -----------

Not reading Portuguese, I have two comments.

 - How well does AsciiDoc and its manpage backend work with these standard
   section names localized?

 - The length of the underline must match the section header word it
   underlines.

Has anybody actually tried to format this document, either before or after
your patch?

^ permalink raw reply

* Re: [PATCH RESEND] git submodule add: make the <path> parameter optional
From: Junio C Hamano @ 2009-10-05  1:31 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Jens Lehmann, Lars Hjemli, git
In-Reply-To: <alpine.DEB.1.00.0910042304060.4985@pacific.mpi-cbg.de>

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

> So far, I started submodules by cloning them, doing everything in the 
> other files needed to integrate, and then actually wondered why "git 
> submodule add" could not simply take the (relative) path to the 
> checked-out submodule and deduce the URL from the corresponding config?

Let me try to rephrase the above to see if I understand what you are
doing.  When building a top-level superproject that uses two submodules
from other places, you would do:

	$ git init toplevel
        $ cd toplevel
        $ git clone $overthere submodule1
        $ git clone $elsewhere submodule2
        $ edit Makefile common.h
        $ git add Makefile common.h submodule1 submodule2

and by "the corresponding config", you mean "submodule1/.git/config
already records that it came from $overthere, and there is no reason to
say it again in 'git submodule add $overthere submodule1'".

If that is the case, I think it is a very sane argument.  It also makes me
wonder why we need "git submodule add" as a separate step to begin with
(i.e. "git add" Porcelain could do that behind the scene), but that is
probably a separate topic.

> So I would actually vote for making the <repository> parameter optional...

In your "git submodule add submodule1", it would be quite clear that it is
a local path and <repository> is being omitted.  On the other hand, if you
said "git submodule add $overthere" without submodule1, because $overthere
is not likely to be an in-tree path, it also would be clear that it is
omitting the path.  IOW, these two typesavers are not mutually exclusive.

In real life, it is very likely $overthere does _not_ end with submodule1,
and your suggestion would probably be more useful than the patch under
discussion, I think.

^ permalink raw reply

* Re: [PATCH] tests: make all test files executable
From: Mark Rada @ 2009-10-05  1:25 UTC (permalink / raw)
  To: Jeff King; +Cc: Mark Rada, git
In-Reply-To: <20091004134022.GA14209@sigill.intra.peff.net>

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

On 09-10-04 9:40 AM, Jeff King wrote:
> On Sun, Oct 04, 2009 at 09:18:20AM -0400, Mark Rada wrote:
> 
>>> Ah, nevermind. The problem is that your patch was word-wrapped, making
>>> the second "diff --git" line bogus. It would have been nice to have it
>>> print a warning instead of silently ignoring that bit of the patch.
>>>
>> I didn't have format=flowed buggering things up this time, so I don't
>> quite understand the problem; could you please explain with more
>> details?
> 
> Sure. The patch is perfect except for one line. What should have been:
> 
> diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
> 
> was wrapped to:
> 
> diff --git a/t/t9501-gitweb-standalone-http-status.sh
> b/t/t9501-gitweb-standalone-http-status.sh
> 
> I have no idea how you did that, though. :)
> 
> It looks like you send with Thunderbird. How do you get the diff content
> into the email? Is it possible that it wraps the content after you have
> gotten it there?

I don't think so, I have plug-in that disables wrapping and has worked
just fine for the last couple of patches I sent, so I'm not sure what
was going on there.

>> When I try to apply the patch from a saved copy of the e-mail, I get
>> the following error:
>>
>> 	# git am ~/Downloads/\[PATCH\]\ tests_\ make\ all\ test\ files\
>> executable.eml
>> 	Patch format detection failed.
>> 	zsh: exit 1     git am
>>
>> The difference between the patch created by format-patch and the saved
>> e-mail is just some e-mail header information. Is that a different error
>> than what you were getting? I'm not sure what I'm doing wrong here, help
>> would be appreciated.
> 
> Yeah, that's totally different than the problem I was having. I save to
> an mbox from mutt, which "git am" understands just fine. I'd have to see
> what was in your .eml file to know why "git am" couldn't figure it out
> (and it might be a good test case, as "git am" has recently learned to
> accept more mailbox formats).

I've attached a copy of the .eml file.


-- 
Mark Rada (ferrous26)
marada@uwaterloo.ca

[-- Attachment #2: [PATCH] tests_ make all test files executable.eml --]
[-- Type: message/rfc822, Size: 1142 bytes --]

Subject: 

From:
Mark Rada <marada@mailservices.uwaterloo.ca>
Date:
Thu, 01 Oct 2009 21:59:20 -0400
To:
Junio C Hamano <gitster@pobox.com>
CC:
git@vger.kernel.org

Rm9yIGNvbnNpc3RlbmN5IHdpdGggdGhlIHJlc3Qgb2YgdGhlIHRlc3QgZmlsZXMuCgpTaWdu
ZWQtb2ZmLWJ5OiBNYXJrIFJhZGEgPG1hcmFkYUB1d2F0ZXJsb28uY2E+Ci0tLQoKT24gMjAw
OS0xMC0wMSwgYXQgNDoxMyBBTSwgSmFrdWIgTmFyZWJza2kgd3JvdGU6Cj4+ID4+IGRpZmYg
LS1naXQgYS90L3Q5NTAxLWdpdHdlYi1zdGFuZGFsb25lLWh0dHAtc3RhdHVzLnNoIGIvdC90
OTUwMS1naXR3ZWItc3RhbmRhbG9uZS1odHRwLXN0YXR1cy5zaAo+PiA+PiBpbmRleCBkMGZm
MjFkLi4wNjg4YTU3IDEwMDY0NAo+PiA+PiAtLS0gYS90L3Q5NTAxLWdpdHdlYi1zdGFuZGFs
b25lLWh0dHAtc3RhdHVzLnNoCj4+ID4+ICsrKyBiL3QvdDk1MDEtZ2l0d2ViLXN0YW5kYWxv
bmUtaHR0cC1zdGF0dXMuc2gKPiA+IAo+ID4gQlRXLiB0aGUgcmVzdCBvZiB0ZXN0IHNjcmlw
dHMgYXJlIGV4ZWN1dGFibGUsIGJ1dCBub3QgdGhpcyBvbmU/IFdoeT8KPiA+IChCdXQgY29y
cmVjdGluZyB0aGlzIHNob3VsZCBiZSBkb25lLCBpZiBuZWVkZWQsIGluIHNlcGFyYXRlIGNv
bW1pdCkuCgpJIG5vdGljZWQgb25lIG90aGVyIHRlc3Qgc2NyaXB0IHRoYXQgd2FzIG5vdCBz
ZXQgdG8gYmUgZXhlY3V0YWJsZS4KCgogMCBmaWxlcyBjaGFuZ2VkLCAwIGluc2VydGlvbnMo
KyksIDAgZGVsZXRpb25zKC0pCiBtb2RlIGNoYW5nZSAxMDA2NDQgPT4gMTAwNzU1IHQvdDU1
MzEtZGVlcC1zdWJtb2R1bGUtcHVzaC5zaAogbW9kZSBjaGFuZ2UgMTAwNjQ0ID0+IDEwMDc1
NSB0L3Q5NTAxLWdpdHdlYi1zdGFuZGFsb25lLWh0dHAtc3RhdHVzLnNoCgpkaWZmIC0tZ2l0
IGEvdC90NTUzMS1kZWVwLXN1Ym1vZHVsZS1wdXNoLnNoIGIvdC90NTUzMS1kZWVwLXN1Ym1v
ZHVsZS1wdXNoLnNoCm9sZCBtb2RlIDEwMDY0NApuZXcgbW9kZSAxMDA3NTUKZGlmZiAtLWdp
dCBhL3QvdDk1MDEtZ2l0d2ViLXN0YW5kYWxvbmUtaHR0cC1zdGF0dXMuc2gKYi90L3Q5NTAx
LWdpdHdlYi1zdGFuZGFsb25lLWh0dHAtc3RhdHVzLnNoCm9sZCBtb2RlIDEwMDY0NApuZXcg
bW9kZSAxMDA3NTUKLS0gMS42LjUucmMyIA==

^ permalink raw reply

* Re: [PATCH] Reserve a slot for argv[0] in default_arg.
From: Junio C Hamano @ 2009-10-04 22:20 UTC (permalink / raw)
  To: Jeff King; +Cc: Petter Urkedal, git, Stephen Boyd
In-Reply-To: <20091004182746.GA22995@coredump.intra.peff.net>

Stephen Boyd <bebarino@gmail.com> writes:

> On Sun, 2009-10-04 at 14:27 -0400, Jeff King wrote:
>> 
>> Ah, thanks, for some reason I wasn't able to produce it before, but I
>> can easily replicate it here. I think it's a regression from converting
>> show-branch to use parse_options, which happened in May, but I didn't
>> actually bisect it. I'm not sure showbranch.default has worked at all
>> since then (which I guess goes to show how many people are actually
>> using it).

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?

> Correct. Junio sent a patch to fix this problem in June[1]. I guess he
> must have dropped his own patch, or he wasn't satisfied with how parse
> options clobbers things.
>
> [1] http://article.gmane.org/gmane.comp.version-control.git/121142

I had it kept still in my Inbox; thanks for noticing.  Petter's patch does
essentially the same thing, but the old patch had a better log message
that described where in the history the fix should apply, so I'd probably
use that with your test squashed in.

Thanks.

^ permalink raw reply

* Re: [PATCH] Add the --submodule-summary option to the diff option family
From: Junio C Hamano @ 2009-10-04 22:19 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Jens Lehmann
In-Reply-To: <67a884457aeaead275612be10902a80726b2a7db.1254668669u.git.johannes.schindelin@gmx.de>

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

> Now you can see the submodule summaries inlined in the diff, instead of
> not-quite-helpful SHA-1 pairs.

This looks useful, but I do not think this is --summary.  It is not part
of the "summary" output but is about making output for Submoodules more
verbose.  I'd suggest naming it --verbose-submodule or something.

> The format imitates what "git submodule summary" shows.

The output format needs to be described better here and also in
Documentation/diff-format.txt.

> To do that, <path>/.git/objects/ is added to the alternate object
> databases (if that directory exists).

Is it always true that <path>/.git is the GIT_DIR for that submodule?  Not
complaining but checking sanity.

> diff --git a/diff.c b/diff.c
> index 9e00131..cdef322 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -1557,6 +1558,17 @@ static void builtin_diff(const char *name_a,
>  	const char *a_prefix, *b_prefix;
>  	const char *textconv_one = NULL, *textconv_two = NULL;
>  
> +	if (DIFF_OPT_TST(o, SUMMARIZE_SUBMODULES) &&
> +			(!one->mode || S_ISGITLINK(one->mode)) &&
> +			(!two->mode || S_ISGITLINK(two->mode))) {
> +		const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
> +		const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
> +		show_submodule_summary(o->file, one ? one->path : two->path,
> +				one->sha1, two->sha1,
> +				del, add, reset);
> +		return;
> +	}
> +

Isn't this "textual diff" codepath?

I would have expected --submodule-summary output to be near the --summary
output and to be generated in the same codepath to generate it; if this
were named --verbose-submodule, I would certainly understand it, and I
think the placement is saner in the textual diff.

> diff --git a/submodule.c b/submodule.c
> new file mode 100644
> index 0000000..3f2590d
> --- /dev/null
> +++ b/submodule.c
> @@ -0,0 +1,108 @@
> +#include "cache.h"
> +#include "submodule.h"
> +#include "dir.h"
> +#include "diff.h"
> +#include "commit.h"
> +#include "revision.h"
> +
> +int add_submodule_odb(const char *path)
> +{
> +	struct strbuf objects_directory = STRBUF_INIT;
> +	struct alternate_object_database *alt_odb;
> +
> +	strbuf_addf(&objects_directory, "%s/.git/objects/", path);
> +	if (!is_directory(objects_directory.buf))
> +		return -1;
> +
> +	/* avoid adding it twice */
> +	for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next)
> +		if (alt_odb->name - alt_odb->base == objects_directory.len &&
> +				!strncmp(alt_odb->base, objects_directory.buf,
> +					objects_directory.len))
> +			return 0;
> +	alt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb));
> +	alt_odb->next = alt_odb_list;
> +	strcpy(alt_odb->base, objects_directory.buf);
> +	alt_odb->name = alt_odb->base + objects_directory.len;
> +	alt_odb->name[2] = '/';
> +	alt_odb->name[40] = '\0';
> +	alt_odb->name[41] = '\0';
> +	alt_odb_list = alt_odb;
> +	prepare_alt_odb();

The lines after "avoid adding it twice" look somewhat familiar.

Don't we already have other codepaths to add an alternate odb that need to
be careful in the same way (i.e. do not add twice, do not loop, etc.), and
if so don't you want to reuse it after refactoring?

> +void show_submodule_summary(FILE *f, const char *path,
> +		unsigned char one[20], unsigned char two[20],
> +		const char *del, const char *add, const char *reset)
> +{
> +	struct rev_info rev;
> +	struct commit *commit, *left, *right;
> +	struct commit_list *merge_bases, *list;
> +	const char *message = NULL;
> +	struct strbuf sb = STRBUF_INIT;
> +	static const char *format = "    %m %s";
> +	int fast_forward = 0, fast_backward = 0;
> +
> +	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)";

Are you sure about this?  Wouldn't "git diff HEAD" (not looking at the
index) give you the 0{40} on the "two" side when the path is modified?

> +	if (!message) {
> +		init_revisions(&rev, NULL);
> +		setup_revisions(0, NULL, &rev, NULL);
> +		rev.left_right = 1;
> +		left->object.flags |= SYMMETRIC_LEFT;
> +		add_pending_object(&rev, &left->object, path);
> +		add_pending_object(&rev, &right->object, path);
> +		merge_bases = get_merge_bases(left, right, 1);
> +		if (merge_bases) {
> +			if (merge_bases->item == left)
> +				fast_forward = 1;
> +			else if (merge_bases->item == right)
> +				fast_backward = 1;
> +		}
> +		for (list = merge_bases; list; list = list->next) {
> +			list->item->object.flags |= UNINTERESTING;
> +			add_pending_object(&rev, &list->item->object,
> +				sha1_to_hex(list->item->object.sha1));
> +		}
> +		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?

> +	}
> +
> +	strbuf_addf(&sb, "Submodule %s %s..", path,
> +			find_unique_abbrev(one, DEFAULT_ABBREV));
> +	if (!fast_backward && !fast_forward)
> +		strbuf_addch(&sb, '.');
> +	strbuf_addf(&sb, "%s", find_unique_abbrev(two, DEFAULT_ABBREV));
> +	if (message)
> +		strbuf_addf(&sb, " %s\n", message);
> +	else
> +		strbuf_addf(&sb, "%s:\n", fast_backward ? " (rewind)" : "");

No corresponding "(fast-forward)" label?

> +	fwrite(sb.buf, sb.len, 1, f);
> +
> +	if (!message) {
> +		while ((commit = get_revision(&rev))) {
> +			strbuf_setlen(&sb, 0);
> +			if (del)
> +				strbuf_addstr(&sb, commit->object.flags &
> +						SYMMETRIC_LEFT ? del : add);
> +			format_commit_message(commit, format, &sb,
> +					rev.date_mode);
> +			if (del)
> +				strbuf_addstr(&sb, reset);

Three points, two of which are minor.

 - Checking "del" to decide if you want to say "reset" feels funny.

 - In the "ANSI-terminal only" world view, adding colors to strbuf and
   writing it out together with the actual strings is an easy thing to do.
   Don't Windows folks have trouble converting this kind of code to their
   color control call that is separate from writing strings out?  If it is
   not a problem, I do not have any objection to it, but otherwise I'd
   suggest not to add any more code that stores color escape sequence in
   strbuf, so that we would not make later conversion by Windows folks
   harder than necessary.

 - The output is similar but different from "submodule --summary" and
   there is no justification described in the patch nor the proposed
   commit log message.

   o Why does "submodule --summary" use --first-parent and this patch
     doesn't?

   o "submodule --summary" allows users to limit the walk but this seems
     to give the full list---is it useful (or even sane) to always give
     a full list?

> +			strbuf_addch(&sb, '\n');
> +			fwrite(sb.buf, sb.len, 1, f);
> +		}
> +		clear_commit_marks(left, ~0);
> +		clear_commit_marks(right, ~0);
> +	}

Aren't object flags shared between the outer revision walker that walks
the log and this new walker?  If the supermodule history and submodule
history share commits (e.g. when later git.git starts using submodules to
bind gitk and git-gui), wouldn't this break "git log -p" a big way?

In any case, thanks for an interesting patch.  Looking forward to see how
this topic evolves.

^ permalink raw reply

* Re: [PATCH] Teach 'rebase -i' the command "amend"
From: Johannes Schindelin @ 2009-10-04 21:09 UTC (permalink / raw)
  To: Björn Gustavsson; +Cc: git, gitster
In-Reply-To: <4AC8F22F.5070101@gmail.com>

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

Hi,

On Sun, 4 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 "amend" command.

I thought that we had a discussion about this and that the consensus was 
that "amend" would be misleading.  Maybe you can find that thread in 
GMane?

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH RESEND] git submodule add: make the <path> parameter optional
From: Johannes Schindelin @ 2009-10-04 21:05 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Junio C Hamano, Lars Hjemli, git
In-Reply-To: <4AC8E0A8.4000901@web.de>

Hi,

On Sun, 4 Oct 2009, Jens Lehmann wrote:

> Junio C Hamano schrieb:
> > Jens Lehmann <Jens.Lehmann@web.de> writes:
> > 
> >> When <path> is not given, use the "humanish" part of the source repository
> >> instead.
> >>
> >> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> >> ---
> >>
> >> With this patch, git submodule add behaves like git clone in this respect.
> >>
> >> Didn't get a response the last weeks, so here is a resend.
> >>
> >>
> >>  Documentation/git-submodule.txt |    8 ++++++--
> >>  git-submodule.sh                |    7 ++++++-
> >>  t/t7400-submodule-basic.sh      |   16 ++++++++++++++++
> >>  3 files changed, 28 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> >> index 5ccdd18..4ef70c4 100644
> >> --- a/Documentation/git-submodule.txt
> >> +++ b/Documentation/git-submodule.txt
> >> @@ -10,7 +10,7 @@ SYNOPSIS
> >>  --------
> >>  [verse]
> >>  'git submodule' [--quiet] add [-b branch]
> >> -	      [--reference <repository>] [--] <repository> <path>
> >> +	      [--reference <repository>] [--] <repository> [<path>]
> >>  'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
> >>  'git submodule' [--quiet] init [--] [<path>...]
> >>  'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
> >> @@ -69,7 +69,11 @@ add::
> >>  	to the changeset to be committed next to the current
> >>  	project: the current project is termed the "superproject".
> >>  +
> >> -This requires two arguments: <repository> and <path>.
> >> +This requires at least one argument: <repository>. The optional
> >> +argument <path> is the relative location for the cloned submodule
> >> +to exist in the superproject. If <path> is not given, the
> >> +"humanish" part of the source repository is used ("repo" for
> >> +"/path/to/repo.git" and "foo" for "host.xz:foo/.git").
> > 
> > I do not know if this is useful in practice nor even desired.  Comments?
> 
> As nobody commented until now, i'll explain my motivation for this patch.
> 
> When adding submodules i was surprised to find that i had to explicitly
> provide the pathname even though it could be easily generated from the
> reponame as git clone does it. And i see git clone and git submodule add
> as related commands from a users perspective, they both connect a remote
> repo to a working directory.
> 
> IMHO this patch makes the ui more consistent and doesn't break existing
> setups or scripts. And it is really useful because i don't do typos in
> the pathname anymore ;-)

So far, I started submodules by cloning them, doing everything in the 
other files needed to integrate, and then actually wondered why "git 
submodule add" could not simply take the (relative) path to the 
checked-out submodule and deduce the URL from the corresponding config?

So I would actually vote for making the <repository> parameter optional...

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Reserve a slot for argv[0] in default_arg.
From: Stephen Boyd @ 2009-10-04 20:02 UTC (permalink / raw)
  To: Jeff King; +Cc: Petter Urkedal, git, gitster
In-Reply-To: <20091004182746.GA22995@coredump.intra.peff.net>

On Sun, 2009-10-04 at 14:27 -0400, Jeff King wrote:
> 
> Ah, thanks, for some reason I wasn't able to produce it before, but I
> can easily replicate it here. I think it's a regression from converting
> show-branch to use parse_options, which happened in May, but I didn't
> actually bisect it. I'm not sure showbranch.default has worked at all
> since then (which I guess goes to show how many people are actually
> using it).

Correct. Junio sent a patch to fix this problem in June[1]. I guess he
must have dropped his own patch, or he wasn't satisfied with how parse
options clobbers things.

[1] http://article.gmane.org/gmane.comp.version-control.git/121142

^ permalink raw reply

* [PATCH] Teach 'rebase -i' the command "amend"
From: Björn Gustavsson @ 2009-10-04 19:06 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 "amend" command.

Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
---
Here is my first ever patch of git.

I find that I often use 'git rebase -i' just to correct typos
in my commit messages or to fill in more details, so I thought
it would be a good idea to add to add an "amend" command to
'git rebase -i'.

/Björn

 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..28b90ec 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 "amend".
+
 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..3f6bcc0 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"
 		;;
+	amend|a)
+		comment_for_reflog amend
+
+		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
+#  a, amend = 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..2ca0481 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 "amend", 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|amend)
 		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..3520480 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 'amend' '
+	git checkout -b amend-branch master &&
+	FAKE_LINES="1 2 3 amend 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 amend 3 4" FAKE_COMMIT_MESSAGE="D changed" git rebase -i A &&
+	git show HEAD^ | grep "D changed" &&
+	FAKE_LINES="amend 1 2 3 4" FAKE_COMMIT_MESSAGE="B changed" git rebase -i A &&
+	git show HEAD~3 | grep "B changed" &&
+	FAKE_LINES="1 amend 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.g117a

^ permalink raw reply related


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