Git development
 help / color / mirror / Atom feed
* 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: "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: "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: [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: [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] 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 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] 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] 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] 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] 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] 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] 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] 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 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] Teach 'rebase -i' the command "amend"
From: Johannes Schindelin @ 2009-10-05  9:17 UTC (permalink / raw)
  To: Björn Gustavsson; +Cc: git, gitster
In-Reply-To: <6672d0160910042308v7280dcadyff97b977bcfe12c3@mail.gmail.com>

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

Hi,

On Mon, 5 Oct 2009, Björn Gustavsson wrote:

> 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"?

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).

AFAIR the naming was the only objection.

Ciao,
Dscho

^ permalink raw reply

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

Hi,

On Mon, 5 Oct 2009, Johannes Sixt wrote:

> 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.

And I find it utterly ugly and unreadable there, too. So this is why I did 
not do it.

Ciao,
Dscho

^ permalink raw reply

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

Hi,

On Mon, 5 Oct 2009, Johannes Schindelin wrote:

> On Sun, 4 Oct 2009, Junio C Hamano wrote:
> 
> >  - Checking "del" to decide if you want to say "reset" feels funny.
> 
> Right.  But I wanted to avoid checking for del, add and reset 
> separately.
> 
> So I was wrong, and will fix.

This is the current interdiff:

-- snipsnap --
diff --git a/submodule.c b/submodule.c
index 3f2590d..b441ea1 100644
--- a/submodule.c
+++ b/submodule.c
@@ -92,15 +92,18 @@ void show_submodule_summary(FILE *f, const char *path,
 	if (!message) {
 		while ((commit = get_revision(&rev))) {
 			strbuf_setlen(&sb, 0);
-			if (del)
-				strbuf_addstr(&sb, commit->object.flags &
-						SYMMETRIC_LEFT ? del : add);
+			if (commit->object.flags & SYMMETRIC_LEFT) {
+				if (del)
+					strbuf_addstr(&sb, del);
+			}
+			else if (add)
+				strbuf_addstr(&sb, add);
 			format_commit_message(commit, format, &sb,
 					rev.date_mode);
-			if (del)
+			if (reset)
 				strbuf_addstr(&sb, reset);
 			strbuf_addch(&sb, '\n');
-			fwrite(sb.buf, sb.len, 1, f);
+			fprintf(f, "%s", sb.buf);
 		}
 		clear_commit_marks(left, ~0);
 		clear_commit_marks(right, ~0);

^ permalink raw reply related

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

Heya,

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.

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* [PATCH] Make the MSVC projects use PDB/IDB files named after the project
From: Sebastian Schuberth @ 2009-10-05  9:54 UTC (permalink / raw)
  To: git

Instead of having all PDB files for all projects named "vc90.pdb", name them
after the respective project to make the relation more clear (and to avoid name
clashes when copying files around).

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
---
 contrib/buildsystems/Generators/Vcproj.pm |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/contrib/buildsystems/Generators/Vcproj.pm b/contrib/buildsystems/Generators/Vcproj.pm
index be94ba1..cfa74ad 100644
--- a/contrib/buildsystems/Generators/Vcproj.pm
+++ b/contrib/buildsystems/Generators/Vcproj.pm
@@ -178,6 +178,7 @@ sub createLibProject {
 				MinimalRebuild="true"
 				RuntimeLibrary="1"
 				UsePrecompiledHeader="0"
+				ProgramDataBaseFileName="\$(IntDir)\\\$(TargetName).pdb"
 				WarningLevel="3"
 				DebugInformationFormat="3"
 			/>
@@ -244,6 +245,7 @@ sub createLibProject {
 				RuntimeLibrary="0"
 				EnableFunctionLevelLinking="true"
 				UsePrecompiledHeader="0"
+				ProgramDataBaseFileName="\$(IntDir)\\\$(TargetName).pdb"
 				WarningLevel="3"
 				DebugInformationFormat="3"
 			/>
@@ -401,6 +403,7 @@ sub createAppProject {
 				MinimalRebuild="true"
 				RuntimeLibrary="1"
 				UsePrecompiledHeader="0"
+				ProgramDataBaseFileName="\$(IntDir)\\\$(TargetName).pdb"
 				WarningLevel="3"
 				DebugInformationFormat="3"
 			/>
@@ -472,6 +475,7 @@ sub createAppProject {
 				RuntimeLibrary="0"
 				EnableFunctionLevelLinking="true"
 				UsePrecompiledHeader="0"
+				ProgramDataBaseFileName="\$(IntDir)\\\$(TargetName).pdb"
 				WarningLevel="3"
 				DebugInformationFormat="3"
 			/>
-- 
1.6.5.rc2.13.g1be2

^ permalink raw reply related

* Re: [PATCH] tests: make all test files executable
From: Jeff King @ 2009-10-05  9:59 UTC (permalink / raw)
  To: Mark Rada; +Cc: git
In-Reply-To: <4AC94B07.4000803@mailservices.uwaterloo.ca>

On Sun, Oct 04, 2009 at 09:25:27PM -0400, Mark Rada wrote:

> > 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.

I don't know, then. I'm pretty sure it wasn't wrapped up on my end, as
it is also wrapped in gmane:

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

Maybe some hints are here:

  http://kb.mozillazine.org/Plain_text_e-mail_-_Thunderbird

> > 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.

Hmm. The .eml format appears to be some kind of inscrutable binary
format. "git am" certainly won't understand it. I'm not sure how you
created it, but you need to convince Thunderbird to export as an 'mbox'
format (which it clearly understands, as that is its native local
format). I have no idea how to do that, though.

-Peff

^ permalink raw reply

* Re: [PATCH] Make the MSVC projects use PDB/IDB files named after the project
From: Marius Storm-Olsen @ 2009-10-05 10:08 UTC (permalink / raw)
  To: Sebastian Schuberth; +Cc: git
In-Reply-To: <hacfq0$g9k$1@ger.gmane.org>

Sebastian Schuberth said the following on 05.10.2009 11:54:
> Instead of having all PDB files for all projects named "vc90.pdb", name them
> after the respective project to make the relation more clear (and to avoid name
> clashes when copying files around).
> 
> Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>

Acked-by: Marius Storm-Olsen <mstormo@gmail.com>

--
.marius

^ permalink raw reply

* Re: [PATCH v5 2/2] gitweb: append short hash ids to snapshot files
From: Jakub Narebski @ 2009-10-05 10:06 UTC (permalink / raw)
  To: Mark Rada; +Cc: Junio C Hamano, git
In-Reply-To: <4ABE536D.3070705@mailservices.uwaterloo.ca>

I am sorry for being late with review of this patch.

On Sat, 26 Sep 2009, Mark Rada wrote:

> Teach gitweb how to produce nicer snapshot names by only using the
> short hash id.

A few questions (which I think should be answered in this commit message).

First, what was original behaviour of 'snapshot' action?  Did gitweb
always convert 'h' (hash) parameter to full SHA-1?

Second, do you preserve 'snapshot' behavior that it generated archive
which unpacks to the directory with the same name as basename of (proposed)
archive name?  I mean here that "repo-2cc6859e.tar.gz" unpacks to
"repo-2cc6859e/" directory.  I guess it does, but this should be
mentioned in the commit message.

>               If clients make requests using a tree-ish that is not a
> partial or full SHA-1 hash, then the short hash will also be appended
> to whatever they asked for.

Here example would be a good idea.  I guess this means that if one is
requesting for snapshot of 'next' or 'v1.6.0' of 'repo.git' project,
one would get 'repo-next-2cc6859.tar.gz' or 'repo-v1.6.0-2cc6859.tar.gz'
as [proposed] snapshot filename.

> 
> This also includes tests cases for t9502-gitweb-standalone-parse-output.

I am not sure if it shouldn't be rather t9502-gitweb-standalone-snapshot
test; see comments below for whys.

> 
> Signed-off-by: Mark Rada <marada@uwaterloo.ca>
> ---
> 
> 
> 	Changes since v4:
> 		- moved git_get_full_hash into this commit
> 		- changed test case format, suggested by Junio
> 		- explicity request at least a length of 7 for short hashes
> 
> 
>  gitweb/gitweb.perl                        |   40 +++++++++++++++--
>  t/t9502-gitweb-standalone-parse-output.sh |   67 +++++++++++++++++++++++++++++
>  2 files changed, 103 insertions(+), 4 deletions(-)
>  create mode 100644 t/t9502-gitweb-standalone-parse-output.sh
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 8d4a2ae..bc132a5 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1983,14 +1983,39 @@ sub quote_command {
>  
>  # get HEAD ref of given project as hash
>  sub git_get_head_hash {
> +	return git_get_full_hash(shift, 'HEAD');
> +}

Good.  This means that we don't need to change code that do not use
new feature (new subroutines).  It's nice to cater to backward 
compatibility, especially if it is so low cost as this one.

> +
> +sub git_get_full_hash {
>  	my $project = shift;
> +	my $hash = shift;

I think it might be good idea here to default to 'HEAD', i.e.

  +	my $hash = shift || 'HEAD';

>  	my $o_git_dir = $git_dir;
>  	my $retval = undef;
>  	$git_dir = "$projectroot/$project";
> -	if (open my $fd, "-|", git_cmd(), "rev-parse", "--verify", "HEAD") {
> -		my $head = <$fd>;
> +	if (open my $fd, '-|', git_cmd(), 'rev-parse', '--verify', $hash) {
> +		$hash = <$fd>;
>  		close $fd;
> -		if (defined $head && $head =~ /^([0-9a-fA-F]{40})$/) {
> +		if (defined $hash && $hash =~ /^([0-9a-fA-F]{40})$/) {
> +			$retval = $1;
> +		}

I guess that you use "$retval = $1;" instead of just "$retval = $hash;"
because of similarities with git_get_short_hash, isn't it?  Or it is just
following earlier code?

> +	}
> +	if (defined $o_git_dir) {
> +		$git_dir = $o_git_dir;
> +	}
> +	return $retval;
> +}
> +
> +# try and get a shorter hash id
> +sub git_get_short_hash {
> +	my $project = shift;
> +	my $hash = shift;
> +	my $o_git_dir = $git_dir;
> +	my $retval = undef;
> +	$git_dir = "$projectroot/$project";
> +	if (open my $fd, '-|', git_cmd(), 'rev-parse', '--short=7', $hash) {
> +		$hash = <$fd>;
> +		close $fd;
> +		if (defined $hash && $hash =~ /^([0-9a-fA-F]{7,})$/) {
>  			$retval = $1;
>  		}
>  	}

Note that git_get_full_hash (which additionally does verification) and
git_get_short_hash share much of code.  Perhaps it might be worth to
avoid code duplication somehow?  On the other hand it might be not worth
to complicate code by trying to extract common parts here...

> @@ -5203,6 +5228,13 @@ sub git_snapshot {
>  		die_error(400, 'Object is not a tree-ish');
>  	}
>  
> +
> +	my $full_hash = git_get_full_hash($project, $hash);
> +	if ($full_hash =~ /^$hash/) {
> +		$hash = git_get_short_hash($project, $hash);
> +	} else {
> +		$hash .= '-' . git_get_short_hash($project, $hash);
> +	}

I think we might want to avoid calling git_get_full_hash (and extra call
to "git rev-parse" command, which is extra fork) if we know in advance
that  $full_hash =~ /^$hash/  can't be true, i.e. if $hash doesn't match
/^[0-9a-fA-F]+$/.  That would require that we continue to use $hash
and not $full_hash, see comment for the chunk below.

BTW do you think that having better name (nicer name in the case
when $hash is full SHA-1, or name which describes exact version as 
in the case when $hash is branch name or just 'HEAD') is worth
slight extra cost of "git rev-parse --abbrev=7"?

>  	my $name = $project;
>  	$name =~ s,([^/])/*\.git$,$1,;
>  	$name = basename($name);
> @@ -5213,7 +5245,7 @@ sub git_snapshot {
>  	$cmd = quote_command(
>  		git_cmd(), 'archive',
>  		"--format=$known_snapshot_formats{$format}{'format'}",
> -		"--prefix=$name/", $hash);
> +		"--prefix=$name/", $full_hash);

Why this change?

>  	if (exists $known_snapshot_formats{$format}{'compressor'}) {
>  		$cmd .= ' | ' . quote_command(@{$known_snapshot_formats{$format}{'compressor'}});
>  	}
> diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh
> new file mode 100644
> index 0000000..5f2b1d5
> --- /dev/null
> +++ b/t/t9502-gitweb-standalone-parse-output.sh
> @@ -0,0 +1,67 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2009 Mark Rada
> +#
> +
> +test_description='gitweb as standalone script (parsing script output).
> +
> +This test runs gitweb (git web interface) as a CGI script from the
> +commandline, and checks that it produces the correct output, either
> +in the HTTP header or the actual script output.'

Currently all tests here are about 'snapshot' action.  They are quite
specific, and they do require some knowledge about chosen archive format.
I think it would be better to put snapshot test into separate test,
i.e. in 't/t9502-gitweb-standalone-snapshot.sh'.

> +
> +
> +. ./gitweb-lib.sh
> +
> +# ----------------------------------------------------------------------
> +# snapshot file name
> +
> +test_commit \
> +	'SnapshotFileTests' \
> +	'i can has snapshot?'

Errr... with filename [cutely] called 'i can has snapshot?' you would
have, I guess, problems with tests on MS Windows, where IIRC '?' is
forbidden in filenames.

Perhaps

  +test_commit \
  +	'Initial commit' \
  +	'foo'

or

  +test_commit \
  +	'SnapshotFileTests' \
  +	'foo' 'i can has snapshot?'


> +

In the test below you use "git rev-parse --verify HEAD" and
"git rev-parse --short HEAD" over and over.  I think it would be better
to calculate them upfront:

  +test_expect_success 'calculate full and short ids' '
  +	FULLID= $(git rev-parse --verify  HEAD) &&
  +	SHORTID=$(git rev-parse --short=7 HEAD)
  +'

> +test_expect_success 'snapshots: give full hash' '

You test here that giving full has works, and that gitweb uses short hash
in file name.  Better name would therefore be something like

  +test_expect_success 'snapshots: give full hash, get short hash' '


> +	ID=`git rev-parse --verify HEAD` &&
> +	gitweb_run "p=.git;a=snapshot;h=$ID;sf=tgz" &&

Here I had to remember that 'tgz' snapshot format is enabled by default.
I think it could be better to explicitly enable it in preparation step.

> +	ID=`git rev-parse --short HEAD` &&
> +	grep ".git-$ID.tar.gz" gitweb.output

Here had to think a bit that gitweb.output consists both of HTTP headers,
and of response body, and you are grepping here in the HTTP headers part.
It would be better solution for gitweb_run to split gitweb.output into
gitweb.headers and gitweb.body (perhaps if requested by setting some
variable, e.g. GITWEB_SPLIT_OUTPUT).

It can be done using the following lines:

	sed    -e '/^\r$/'      <gitweb.output >gitweb.headers
	sed -n -e '0,/^\r$/!p'  <gitweb.output >gitweb.body

	# gitweb.headers is used to parse http headers
	# gitweb.body is response without http headers

But the second one uses GNU sed extension; I don't know how to write
it in more portable way.

Then

> +	grep ".git-$ID.tar.gz" gitweb.output

would be

  +	grep ".git-$ID.tar.gz" gitweb.headers

Note that this would mean that t/t9501-gitweb-standalone-http-status.sh
should also be updated to use gitweb.headers and gitweb.body

> +'
> +test_debug 'cat gitweb.output'

Not a good idea in current state.  gitweb.output contains binary part,
and generally it is not a good idea to output binary files (which can
contain ANSI escape sequences) to terminal.

> +
> +test_expect_success 'snapshots: give short hash' '

  +test_expect_success 'snapshots: give short hash, get short hash' '

> +	ID=`git rev-parse --short HEAD` &&

Gitweb uses '--short=7'.  Shouldn't you use the same option here?

> +	gitweb_run "p=.git;a=snapshot;h=$ID;sf=tgz" &&
> +	grep ".git-$ID.tar.gz" gitweb.output
> +'
> +test_debug 'cat gitweb.output'
> +
> +test_expect_success 'snapshots: give almost full hash' '

  +test_expect_success 'snapshots: give almost full hash, get short hash' '

> +	ID=`git rev-parse --short=30 HEAD` &&
> +	gitweb_run "p=.git;a=snapshot;h=$ID;sf=tgz" &&
> +	ID=`git rev-parse --short HEAD` &&
> +	grep ".git-$ID.tar.gz" gitweb.output
> +'
> +test_debug 'cat gitweb.output'
> +
> +test_expect_success 'snapshots: give HEAD tree-ish' '

  +test_expect_success 'snapshots: give HEAD, get HEAD-<short hash>' '

> +	gitweb_run "p=.git;a=snapshot;h=HEAD;sf=tgz" &&
> +	ID=`git rev-parse --short HEAD` &&
> +	grep ".git-HEAD-$ID.tar.gz" gitweb.output
> +'
> +test_debug 'cat gitweb.output'
> +
> +test_expect_success 'snapshots: give branch name tree-ish' '

  +test_expect_success 'snapshots: give branch name, get <branch name>-<short hash>' '

> +	gitweb_run "p=.git;a=snapshot;h=master;sf=tgz" &&
> +	ID=`git rev-parse --short master` &&
> +	grep ".git-master-$ID.tar.gz" gitweb.output
> +'
> +test_debug 'cat gitweb.output'
> +
> +test_expect_success 'snapshots: give tag tree-ish' '

  +test_expect_success 'snapshots: give tag name, get <tag name>-<short hash>' '

> +	gitweb_run "p=.git;a=snapshot;h=SnapshotFileTests;sf=tgz" &&
> +	ID=`git rev-parse --short SnapshotFileTests` &&
> +	grep ".git-SnapshotFileTests-$ID.tar.gz" gitweb.output
> +'
> +test_debug 'cat gitweb.output'

Note that to avoid ambiguities currently gitweb uses refs/heads/master
and refs/tags/SnapshotFileTests... but dealing with this issue should be
left, I think, for separate commit.

> +
> +
> +test_done
> -- 
> 1.6.4.GIT
> 
> 
> 

-- 
Jakub Narebski
Poland

^ permalink raw reply

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

On Sun, Oct 04, 2009 at 06:53:49PM -0700, Junio C Hamano wrote:

> 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?

No, I didn't, and I should have when I picked up the patch in the first
place. You are right, asciidoc barfs (both for html and xml generation):

  ERROR: gittutorial.txt: line 5: first section must be named NAME
  ERROR: gittutorial.txt: line 9: second section must be named SYNOPSIS

-Peff

^ permalink raw reply

* [PATCH] Speedup bash completion loading
From: Kirill Smelkov @ 2009-10-05 10:03 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Kirill Smelkov

On my slow laptop (P3 700MHz), system-wide bash completions take too
much time to load (> 1s), and significant fraction of this time is spent
loading git-completion.bash:

    $ time bash -c '. git-completion.bash'  # before this patch

    real    0m0.317s
    user    0m0.250s
    sys     0m0.060s

I've tracked down that the most time is spent warming up merge_strategy,
all_command & porcelain_command caches.

Since git is not used in each and every interactive xterm, I think it
would be perfectly ok to load completion support with cold caches, and
then load needed thing lazily.

As __git_merge_stratiegies(), __git_all_commands() &
__git_porcelain_command() already cache their results, we can safely
remove associated cache initialization code and be done with it:

    $ time bash -c '. git-completion.bash'  # after this patch

    real    0m0.069s
    user    0m0.050s
    sys     0m0.020s

Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru>
---
 contrib/completion/git-completion.bash |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

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

^ 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