Git development
 help / color / mirror / Atom feed
* Re: Python extension commands in git - request for policy change
From: Joshua Jensen @ 2012-12-12  5:15 UTC (permalink / raw)
  To: Patrick Donnelly
  Cc: Nguyen Thai Ngoc Duy, Tomas Carnecky, Sitaram Chamarty,
	Michael Haggerty, Felipe Contreras, Eric S. Raymond, git
In-Reply-To: <CACh33FqTBOMar=V8=aiE2asZ_ri37fAqeghJLqGecYk=qtvBiQ@mail.gmail.com>

----- Original Message -----
From: Patrick Donnelly
Date: 12/11/2012 7:26 PM
>> If we use lua for writing "builtin" commands,
>> we'll need to export a lot of C functions and writing wrappers like
>> this is boring and time consuming. Also, assume I export fn(char*,int)
>> to Lua, then I change the prototype to fn(char*, char*), can Lua spot
>> all the call sites at compile time (or something) so I can update
>> them?
> If the API calls are generic (don't require special handling), you can
> use some preprocessor magic to save time/space.
Since I mostly use C++, thanks to template metaprogramming, I get to 
register functions like so:

         void SomeExistingFunction(char* str, int num);

luaState->GetGlobals().RegisterDirect("SomeExistingFunction", 
SomeExistingFunction);

I certainly am not suggesting C++ be used within Git, but in this case, 
C++ has some nice compile-time advantages.

:)

-Josh

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Joshua Jensen @ 2012-12-12  5:11 UTC (permalink / raw)
  To: esr
  Cc: Sitaram Chamarty, Patrick Donnelly, Nguyen Thai Ngoc Duy,
	Michael Haggerty, Felipe Contreras, git
In-Reply-To: <20121212033043.GA24937@thyrsus.com>

----- Original Message -----
From: Eric S. Raymond
Date: 12/11/2012 8:30 PM
> It might be a good fit for extending git; I wouldn't be very surprised if
> that worked. However, I do have concerns about the "Oh, we'll just
> lash together a binding to C" attitude common among lua programmers; I
> foresee maintainability problems and the possibility of slow death by
> low-level details as that strategy tries to scale up.
I don't understand this statement: "Oh, we'll just lash together a 
binding to C" attitude.

??
> My sense is that git's use cases are better served by a glue language
> in the Python/Perl/Ruby class rather than an extension langage. But
> my mind is open on this issue.
I spend nearly 100% of my Git time on Windows.

Spawning new processes in Windows is dog slow.  Using 'git rebase', 
arguably my favorite Git command, is time-waiting torture.  I'm also on 
about as fast of a Windows machine as money can buy these days.

I have a Git add-on similar to git-media that uses the smudge and clean 
filters to read/write large binary files into a separate storage 
location.  When checking out a workspace, Git shells out to run a filter 
for each file it needs to write to the workspace.

I can get a maximum of 100 processes per second with this technique, 
resulting in just 100 files being written to disk.  However, I tend to 
see closer to 60 files written to disk.

So, I patched Git to allow the smudge/clean filters to load up a DLL 
that executes a Lua script.  The Lua script properly retrieves+caches a 
file locally, or it puts the file on a network share.

The in-process DLL checkout ends up being every bit as fast as when we 
use Perforce to sync files to our local workspace.  Git, then, can be a 
Perforce replacement for our needs.

(For those who don't know, Perforce handles large workspaces with 
massive binary files very efficiently.)

Anyway, my preference is to allow scripts to run in-process within Git, 
because it is far, far faster on Windows.  I imagine it is faster than 
forking processes on non-Windows machines, too, but I have no statistics 
to back that up.

Python, Perl, or Ruby can be embedded, too, but Lua probably embeds the 
easiest and smallest out of those other 3 languages.

And shell scripts tend to be the slowest on Windows due to the excessive 
numbers of process invocations needed to get anything reasonable done.

-Josh

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Tomas Carnecky @ 2012-12-12  2:22 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy
  Cc: Sitaram Chamarty, Patrick Donnelly, Michael Haggerty,
	Felipe Contreras, Eric S. Raymond, git
In-Reply-To: <CACsJy8A9h4QJ_iWvQqTtYa4NPH6Q1Gy0NTozbgukC3=ep58mLA@mail.gmail.com>

On Wed, 12 Dec 2012 08:50:27 +0700, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> On Wed, Dec 12, 2012 at 7:53 AM, Tomas Carnecky
> <tomas.carnecky@gmail.com> wrote:
> > If it doesn't, it would be trivial to add. It's a one-liner. It's been a while
> > since I used Lua, but it would be something like this:
> >
> >     void L_putenv(lua_State *L) {
> >         putenv(lua_tostring(L, 1));
> >     }
> >
> > and then somewhere during setup:
> >
> >     lua_register(L, "putenv", L_putenv);
> 
> I should have done my homework before asking, but well.. is there any
> way to automate this? If we use lua for writing "builtin" commands,
> we'll need to export a lot of C functions and writing wrappers like
> this is boring and time consuming. Also, assume I export fn(char*,int)
> to Lua, then I change the prototype to fn(char*, char*), can Lua spot
> all the call sites at compile time (or something) so I can update
> them?

A Patrick mentioned in an earlier email, there is luaposix which includes lots
of these functions [1]. There may be tools which generate these bindings
automatically, but I'm not aware of any. Likewise, I'm not aware of any static
analyzer which would be required to spot changes in the prototypes (though you
could cover some(most?) of it through tests). The last time I seriously used
Lua and its C bindings was many years ago, so I am not the best person to
answer these questions.

[1]: http://luaposix.github.com/luaposix/docs/index.html

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Eric S. Raymond @ 2012-12-12  3:30 UTC (permalink / raw)
  To: Sitaram Chamarty
  Cc: Patrick Donnelly, Nguyen Thai Ngoc Duy, Michael Haggerty,
	Felipe Contreras, git
In-Reply-To: <CAMK1S_hy8U0rVY=-u-QCqXjhn-6jwz5ofj_q_mbokVn8CGCMtw@mail.gmail.com>

Sitaram Chamarty <sitaramc@gmail.com>:
> [snipping the rest; all valid points no doubt]

I meant to respond to Patrick's post earlier.

I haven't actually written any code in lua yet, but I've read the book;
I think I get it.  I've seen the effects of lua integration on another
large project, Battle for Wesnoth.

I'm not, despite conclusions some people here might have jumped to,
religiously attached to Python.  So I can say this: I think lua as a
language is an *excellent* design.  It is clever, economical,
minimalist, and (other than the one ugly detail of 1-origin indexing)
shows consistent good taste.

It might be a good fit for extending git; I wouldn't be very surprised if
that worked. However, I do have concerns about the "Oh, we'll just
lash together a binding to C" attitude common among lua programmers; I
foresee maintainability problems and the possibility of slow death by
low-level details as that strategy tries to scale up.

And, of course, one problem with calling back into C a lot is that
you walk back into C's resource-management issues.

My sense is that git's use cases are better served by a glue language
in the Python/Perl/Ruby class rather than an extension langage. But
my mind is open on this issue.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Patrick Donnelly @ 2012-12-12  2:26 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy
  Cc: Tomas Carnecky, Sitaram Chamarty, Michael Haggerty,
	Felipe Contreras, Eric S. Raymond, git
In-Reply-To: <CACsJy8A9h4QJ_iWvQqTtYa4NPH6Q1Gy0NTozbgukC3=ep58mLA@mail.gmail.com>

Hi Duy,

On Tue, Dec 11, 2012 at 8:50 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> On Wed, Dec 12, 2012 at 7:53 AM, Tomas Carnecky
> <tomas.carnecky@gmail.com> wrote:
>> If it doesn't, it would be trivial to add. It's a one-liner. It's been a while
>> since I used Lua, but it would be something like this:
>>
>>     void L_putenv(lua_State *L) {
>>         putenv(lua_tostring(L, 1));
>>     }
>>
>> and then somewhere during setup:
>>
>>     lua_register(L, "putenv", L_putenv);
>
> I should have done my homework before asking, but well.. is there any
> way to automate this?

If you want these basic POSIX functions, use an existing library.

If you want to automate adding a number of application specific
functions, you can use swig or similar. AFAIK, all languages rely on
third party tools like swig to assist in automated binding generation.
Although, automated binding generation is usually used to make it easy
to export bindings for multiple languages easily. If Lua is going to
be used as a "standard" module glue language, using swig is really
overkill.

> If we use lua for writing "builtin" commands,
> we'll need to export a lot of C functions and writing wrappers like
> this is boring and time consuming. Also, assume I export fn(char*,int)
> to Lua, then I change the prototype to fn(char*, char*), can Lua spot
> all the call sites at compile time (or something) so I can update
> them?

If the API calls are generic (don't require special handling), you can
use some preprocessor magic to save time/space.

--
- Patrick Donnelly

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Nguyen Thai Ngoc Duy @ 2012-12-12  1:50 UTC (permalink / raw)
  To: Tomas Carnecky
  Cc: Sitaram Chamarty, Patrick Donnelly, Michael Haggerty,
	Felipe Contreras, Eric S. Raymond, git
In-Reply-To: <1355273635-ner-4863@calvin>

On Wed, Dec 12, 2012 at 7:53 AM, Tomas Carnecky
<tomas.carnecky@gmail.com> wrote:
> If it doesn't, it would be trivial to add. It's a one-liner. It's been a while
> since I used Lua, but it would be something like this:
>
>     void L_putenv(lua_State *L) {
>         putenv(lua_tostring(L, 1));
>     }
>
> and then somewhere during setup:
>
>     lua_register(L, "putenv", L_putenv);

I should have done my homework before asking, but well.. is there any
way to automate this? If we use lua for writing "builtin" commands,
we'll need to export a lot of C functions and writing wrappers like
this is boring and time consuming. Also, assume I export fn(char*,int)
to Lua, then I change the prototype to fn(char*, char*), can Lua spot
all the call sites at compile time (or something) so I can update
them?
-- 
Duy

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Tomas Carnecky @ 2012-12-12  0:53 UTC (permalink / raw)
  To: Sitaram Chamarty, Patrick Donnelly
  Cc: Nguyen Thai Ngoc Duy, Michael Haggerty, Felipe Contreras,
	Eric S. Raymond, git
In-Reply-To: <CAMK1S_hy8U0rVY=-u-QCqXjhn-6jwz5ofj_q_mbokVn8CGCMtw@mail.gmail.com>

On Wed, 12 Dec 2012 05:39:43 +0530, Sitaram Chamarty <sitaramc@gmail.com> wrote:
> Does lua have os.putenv() yet?  The inability to even *set* an env var
> before calling something else was a killer for me when I last tried
> it.

If it doesn't, it would be trivial to add. It's a one-liner. It's been a while
since I used Lua, but it would be something like this:

    void L_putenv(lua_State *L) {
        putenv(lua_tostring(L, 1));
    }

and then somewhere during setup:

    lua_register(L, "putenv", L_putenv);

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Patrick Donnelly @ 2012-12-12  0:28 UTC (permalink / raw)
  To: Sitaram Chamarty
  Cc: Nguyen Thai Ngoc Duy, Michael Haggerty, Felipe Contreras,
	Eric S. Raymond, git
In-Reply-To: <CAMK1S_hy8U0rVY=-u-QCqXjhn-6jwz5ofj_q_mbokVn8CGCMtw@mail.gmail.com>

Hi Sitaram,

On Tue, Dec 11, 2012 at 7:09 PM, Sitaram Chamarty <sitaramc@gmail.com> wrote:
> On Tue, Dec 11, 2012 at 11:14 AM, Patrick Donnelly <batrick@batbytes.com> wrote:
>> Lua has been an incredible success for Nmap [2](and other projects).
>> As an embedded scripting language, it's unrivaled in terms of ease of
>> embedding, ease of use for users, and performance. I would strongly
>> recommend the git developers to seriously consider it.
>
> [snipping the rest; all valid points no doubt]
>
> Does lua have os.putenv() yet?  The inability to even *set* an env var
> before calling something else was a killer for me when I last tried
> it.

Lua is pretty strict about being entirely ANSI C and makes very few
exceptions (e.g. dlopen). The exceptions that do exist are only in
base libraries which can easily be thrown out.

> That may make it fine as an embedded language (called *by* something
> else) but it is a bit too "frugal" to use as a glue language (calls
> other things).

As a glue language, this is a feature. The Programming in Lua (written
by the architect of Lua) preface [1] contains the philosophy of Lua on
this issue.

For cases where you do need access to functions like putenv, it's
trivial to either write wrappers for the functions you want to expose
or to incorporate a library that does it all, e.g. luaposix which
contains the majority of POSIX's system calls.

[1] http://www.lua.org/pil/p1.html

--
- Patrick Donnelly

^ permalink raw reply

* Re: Python extension commands in git - request for policy change
From: Sitaram Chamarty @ 2012-12-12  0:09 UTC (permalink / raw)
  To: Patrick Donnelly
  Cc: Nguyen Thai Ngoc Duy, Michael Haggerty, Felipe Contreras,
	Eric S. Raymond, git
In-Reply-To: <CACh33FrGPhaeNzZ2Tj5OxScecOPN13idw8TwU8Mf6o0KsAOB9A@mail.gmail.com>

On Tue, Dec 11, 2012 at 11:14 AM, Patrick Donnelly <batrick@batbytes.com> wrote:
> Sorry I'm late to this party...
>
> I'm an Nmap developer that is casually interested in git development.
> I've been lurking for a while and thought I'd post my thoughts on this
> thread.
>
> On Sun, Nov 25, 2012 at 6:25 AM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
>>> The most important issues to consider when imagining a future with a
>>> hybrid of code in C and some scripting language "X" are:
>>>
>>> * Portability: is "X" available on all platforms targeted by git, in
>>>   usable and mutually-compatible versions?
>>>
>>> * Startup time: Is the time to start the "X" interpreter prohibitive?
>>>   (On my computer, "python -c pass", which starts the Python
>>>   interpreter and does nothing, takes about 24ms.)  This overhead would
>>>   be incurred by every command that is not pure C.
>>>
>>> * Should the scripting language access the C functionality only by
>>>   calling pure-C executables or by dynamically or statically linking to
>>>   a binary module interface?  If the former, then the granularity of
>>>   interactions between "X" and C is necessarily coarse, and "X" cannot
>>>   be used to implement anything but the outermost layer of
>>>   functionality.  If the latter, then the way would be clear to
>>>   implement much more of git in "X" (and lua would also be worth
>>>   considering).
>>>
>>> * Learning curve for developers: how difficult is it for a typical git
>>>   developer to become conversant with "X", considering both (1) how
>>>   likely is it that the typical git developer already knows "X" and
>>>   (2) how straightforward and predictable is the language "X"?
>>>   In this category I think that Python has a huge advantage over
>>>   Perl, though certainly opinions will differ and Ruby would also be
>>>   a contender.
>>
>> * We might also need an embedded language variant, like Jeff's lua
>> experiment. I'd be nice if "X" can also take this role.
>
> Lua has been an incredible success for Nmap [2](and other projects).
> As an embedded scripting language, it's unrivaled in terms of ease of
> embedding, ease of use for users, and performance. I would strongly
> recommend the git developers to seriously consider it.

[snipping the rest; all valid points no doubt]

Does lua have os.putenv() yet?  The inability to even *set* an env var
before calling something else was a killer for me when I last tried
it.

That may make it fine as an embedded language (called *by* something
else) but it is a bit too "frugal" to use as a glue language (calls
other things).

^ permalink raw reply

* Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format
From: Junio C Hamano @ 2012-12-12  0:03 UTC (permalink / raw)
  To: Simon Oosthoek; +Cc: Piotr Krukowiecki, git
In-Reply-To: <7v7goo6vi3.fsf@alter.siamese.dyndns.org>

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

> Perhaps like this?

OK, this time with a log message.

-- >8 --
Subject: [PATCH] git-prompt.sh: update PROMPT_COMMAND documentation

The description of __git_ps1 function operating in two-arg mode was
not very clear.  It said "set PROMPT_COMMAND=__git_ps1" which is not
the right usage for this mode, followed by "To customize the prompt,
do this", giving a false impression that those who do not want to
customize it can get away with no-arg form, which was incorrect.

Make it clear that this mode always takes two arguments, pre and
post, with an example.

The straight-forward one should be listed as the primary usage, and
the confusing one should be an alternate for advanced users.  Swap
the order of these two.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 contrib/completion/git-prompt.sh | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index a8b53ba..9b074e1 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -10,14 +10,20 @@
 #    1) Copy this file to somewhere (e.g. ~/.git-prompt.sh).
 #    2) Add the following line to your .bashrc/.zshrc:
 #        source ~/.git-prompt.sh
-#    3a) In ~/.bashrc set PROMPT_COMMAND=__git_ps1
-#        To customize the prompt, provide start/end arguments
-#        PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
-#    3b) Alternatively change your PS1 to call __git_ps1 as
+#    3a) Change your PS1 to call __git_ps1 as
 #        command-substitution:
 #        Bash: PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
 #        ZSH:  PS1='[%n@%m %c$(__git_ps1 " (%s)")]\$ '
-#        the optional argument will be used as format string
+#        the optional argument will be used as format string.
+#    3b) Alternatively, if you are using bash, __git_ps1 can be
+#        used for PROMPT_COMMAND with two parameters, <pre> and
+#        <post>, which are strings you would put in $PS1 before
+#        and after the status string generated by the git-prompt
+#        machinery.  e.g.
+#           PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
+#        will show username, at-sign, host, colon, cwd, then
+#        various status string, followed by dollar and SP, as
+#        your prompt.
 #
 # The argument to __git_ps1 will be displayed only if you are currently
 # in a git repository.  The %s token will be the name of the current
-- 
1.8.1.rc1.128.gd8d1528

^ permalink raw reply related

* Re: [PATCH] git-prompt: Document GIT_PS1_DESCRIBE_STYLE
From: Junio C Hamano @ 2012-12-11 23:36 UTC (permalink / raw)
  To: Anders Kaseorg; +Cc: git
In-Reply-To: <alpine.DEB.2.00.1212111815580.1093@dr-wily.mit.edu>

Anders Kaseorg <andersk@MIT.EDU> writes:

> GIT_PS1_DESCRIBE_STYLE was introduced in v1.6.3.2~35.  Document it in the 
> header comments.
>
> Signed-off-by: Anders Kaseorg <andersk@mit.edu>
> ---

Thanks.

>  contrib/completion/git-prompt.sh | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
> index bf20491..5ab488c 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -49,6 +49,15 @@
>  # find one, or @{upstream} otherwise.  Once you have set
>  # GIT_PS1_SHOWUPSTREAM, you can override it on a per-repository basis by
>  # setting the bash.showUpstream config variable.
> +#
> +# If you would like to see more information about the identity of
> +# commits checked out as a detached HEAD, set GIT_PS1_DESCRIBE_STYLE
> +# to one of these values:
> +#
> +#     contains      relative to newer annotated tag (v1.6.3.2~35)
> +#     branch        relative to newer tag or branch (master~4)
> +#     describe      relative to older annotated tag (v1.6.3.1-13-gdd42c2f)
> +#     default       exactly matching tag
>  
>  # __gitdir accepts 0 or 1 arguments (i.e., location)
>  # returns location of .git repo

^ permalink raw reply

* [PATCH] git-prompt: Document GIT_PS1_DESCRIBE_STYLE
From: Anders Kaseorg @ 2012-12-11 23:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

GIT_PS1_DESCRIBE_STYLE was introduced in v1.6.3.2~35.  Document it in the 
header comments.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
---
 contrib/completion/git-prompt.sh | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index bf20491..5ab488c 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -49,6 +49,15 @@
 # find one, or @{upstream} otherwise.  Once you have set
 # GIT_PS1_SHOWUPSTREAM, you can override it on a per-repository basis by
 # setting the bash.showUpstream config variable.
+#
+# If you would like to see more information about the identity of
+# commits checked out as a detached HEAD, set GIT_PS1_DESCRIBE_STYLE
+# to one of these values:
+#
+#     contains      relative to newer annotated tag (v1.6.3.2~35)
+#     branch        relative to newer tag or branch (master~4)
+#     describe      relative to older annotated tag (v1.6.3.1-13-gdd42c2f)
+#     default       exactly matching tag
 
 # __gitdir accepts 0 or 1 arguments (i.e., location)
 # returns location of .git repo
-- 
1.8.0

^ permalink raw reply related

* Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format
From: Junio C Hamano @ 2012-12-11 23:04 UTC (permalink / raw)
  To: Simon Oosthoek; +Cc: Piotr Krukowiecki, git
In-Reply-To: <50C7B811.7030006@xs4all.nl>

Simon Oosthoek <s.oosthoek@xs4all.nl> writes:

> This patch for the documentation doesn't seem to be in rc2 of 1.8.1...

There wasn't any patch, and after sending "something like this" I
forgot about the topic, as usual.

> The current tagged 1.8.1-rc2 doesn't yet have your improvement and after
> trying to explain it, I think it should be improved even more ;-)

I am not sure what "my improvement" you are refering to.  Is it only
about how the usage appears in the comment?

Perhaps like this?  I think the straight-forward one should be
listed as the primary usage, and the confusing one should be an
alternate for advanced users, so 3a/3b have been swapped.

 contrib/completion/git-prompt.sh | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git i/contrib/completion/git-prompt.sh w/contrib/completion/git-prompt.sh
index 00fc099..899eb09 100644
--- i/contrib/completion/git-prompt.sh
+++ w/contrib/completion/git-prompt.sh
@@ -10,14 +10,20 @@
 #    1) Copy this file to somewhere (e.g. ~/.git-prompt.sh).
 #    2) Add the following line to your .bashrc/.zshrc:
 #        source ~/.git-prompt.sh
-#    3a) In ~/.bashrc set PROMPT_COMMAND=__git_ps1
-#        To customize the prompt, provide start/end arguments
-#        PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
-#    3b) Alternatively change your PS1 to call __git_ps1 as
+#    3a) Change your PS1 to call __git_ps1 as
 #        command-substitution:
 #        Bash: PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
 #        ZSH:  PS1='[%n@%m %c$(__git_ps1 " (%s)")]\$ '
-#        the optional argument will be used as format string
+#        the optional argument will be used as format string.
+#    3b) Alternatively, if you are using bash, __git_ps1 can be
+#        used for PROMPT_COMMAND with two parameters, <pre> and
+#        <post>, which are strings you would put in $PS1 before
+#        and after the status string generated by the git-prompt
+#        machinery.  e.g.
+#           PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
+#        will show username, at-sign, host, colon, cwd, then
+#        various status string, followed by dollar and SP, as
+#        your prompt.
 #
 # The argument to __git_ps1 will be displayed only if you are currently
 # in a git repository.  The %s token will be the name of the current

^ permalink raw reply related

* Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format
From: Simon Oosthoek @ 2012-12-11 22:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Piotr Krukowiecki, git
In-Reply-To: <7vlidltpyj.fsf@alter.siamese.dyndns.org>

Hi Junio

This patch for the documentation doesn't seem to be in rc2 of 1.8.1...

tonight I was explaining this feature to a small group of people and I
pulled the git tree to get the latest code.

The current tagged 1.8.1-rc2 doesn't yet have your improvement and after
trying to explain it, I think it should be improved even more ;-)

I'm sorry to say I don't have the time right now to make a proper patch,
but I'll try to expand below...

On 28/11/12 21:47, Junio C Hamano wrote:
> Simon Oosthoek <s.oosthoek@xs4all.nl> writes:
> 
>> perhaps the point should read like this:
>> #    3a) In ~/.bashrc set PROMPT_COMMAND
>> #        To customize the prompt, provide start/end arguments
>> #        PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
>>
>> Which would not be confusing at all, I think...
> 
> It says "to customize", so a user who just wants the default (which
> does not exist but the comment does not say so) would be left
> without instruction, no?
> 
>     In $HOME/.bashrc, PROMPT_COMMAND can be set to
>     '__git_ps1 <pre> <post>', where <pre> and <post>
>     are strings you would put in $PS1 before and after
>     the status string generated by git-prompt machinery.
>     e.g.
>         PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
> 
> or something?
> 

The gist should be that:
__git_ps1 can function in two modes; The "traditional" way with command
substitution. This mode is activated when the function is provided 0 or
1 arguments. The new way, activated by providing exactly 2 arguments to
__git_ps1 is intended for use in the PROMPT_COMMAND way of bash to call
a function before writing the prompt. This command can then be (and is
in fact) used to set/overwrite the PS1 variable.

Only in the PROMPT_COMMAND mode can you get a prompt with colors for the
various states of the git tree showing in the prompt.

An example for inclusion in the ~/.bashrc file is:
-----snip-----
PROMPT_COMMAND='__git_ps1 "\u@\h:\w" "\\\$ "'
GIT_PS1_SHOWDIRTYSTATE=true
GIT_PS1_SHOWCOLORHINTS=true
--------------

It turns out that it can be quite confusing to get your head around this
split personality of __git_ps1...

I hope you can somehow get this into the next release with some
improvement to the docs!

Cheers

Simon

^ permalink raw reply

* Re: [PATCH 4/5] pretty: Use mailmap to display username and email
From: Junio C Hamano @ 2012-12-11 22:46 UTC (permalink / raw)
  To: Antoine Pelisse; +Cc: git, Rich Midwinter
In-Reply-To: <1355264493-8298-5-git-send-email-apelisse@gmail.com>

Antoine Pelisse <apelisse@gmail.com> writes:

> Use the mailmap information to display the correct
> username and email address in all log commands.
>
> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
> ---
>  pretty.c | 46 +++++++++++++++++++++++++++++++---------------
>  1 file changed, 31 insertions(+), 15 deletions(-)
>
> diff --git a/pretty.c b/pretty.c
> index 6730add..e232aaa 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -387,6 +387,8 @@ void pp_user_info(const struct pretty_print_context *pp,
>  		  const char *what, struct strbuf *sb,
>  		  const char *line, const char *encoding)
>  {
> +	char person_name[1024];
> +	char person_mail[1024];
>  	struct ident_split ident;
>  	int linelen, namelen;
>  	char *line_end, *date;
> @@ -405,41 +407,55 @@ void pp_user_info(const struct pretty_print_context *pp,
>  	if (split_ident_line(&ident, line, linelen))
>  		return;
>  
> -	namelen = ident.mail_end - ident.name_begin + 1;
> +	memcpy(person_mail, ident.mail_begin, ident.mail_end - ident.mail_begin);
> +	person_mail[ident.mail_end - ident.mail_begin] = 0;
> +
> +	memcpy(person_name, ident.name_begin, ident.name_end - ident.name_begin);
> +	person_name[ident.name_end - ident.name_begin] = 0;
> +
> +	if (pp->mailmap)
> +		map_user(pp->mailmap, person_mail, sizeof(person_mail),
> +			 person_name, sizeof(person_name));

This is why I said that we may want to rethink the API signature of
the map_user() function.  Now this caller has the hardcoded limit
for name and mail parts, and it needs to make copies of strings into
two arrays only because map_user() expects to get two fixed-size
buffers that are to be rewritten in-place.  If we changed map_user()
to take two strbufs (one for name, the other for mail) to be updated
in place, we would still need to make copies, but later code in this
function may be able to lose a few strlen() calls.

Or it might be better to make those two strbufs output-only
parameter, e.g.

	map_user(struct string_list *mailmap,
        	const char *name, size_t namelen,
                const char *mail, size_t maillen,
                struct strbuf *name_out, struct strbuf *mail_out);

then after split_ident_line(), this caller could feed two pointers
into the original "line" as name and mail parameter, without making
any copies (the callee has to make a copy but it has to be done
anyway when the name/mail is mapped).  I suspect it would make this
caller simpler, but I do not know how invasive such changes are for
other callers of map_user().

Such an update can be left outside of this series, of course.

> +		strbuf_addch(sb, ' ');
> +		strbuf_addch(sb, '<');
> +		strbuf_add(sb, person_mail, strlen(person_mail));
> +		strbuf_addch(sb, '>');
>  		strbuf_addch(sb, '\n');

Is that strbuf_addf(sb, " <%s>\n", person_mail)?

^ permalink raw reply

* Re: [PATCH 0/5] Allow git log to use mailmap file
From: Rich Midwinter @ 2012-12-11 22:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Antoine Pelisse, git
In-Reply-To: <7vip886wyr.fsf@alter.siamese.dyndns.org>

It certainly was. Thanks for picking this up so quick guys.

On 11 December 2012 22:33, Junio C Hamano <gitster@pobox.com> wrote:
> Antoine Pelisse <apelisse@gmail.com> writes:
>
>> Implement the feature suggested here [1] by Rich Mindwinter
>> and Junio C Hamano (and following his advices)
>>
>> This is a pre-version so there are a bunch of things still missing,
>> among them:
>>  - There is no tests
>>  - Grep search for mailmap author/committer is not available
>>  - There is no documentation of the new option
>>
>> [1]: http://thread.gmane.org/gmane.comp.version-control.git/211270
>
> That was quick ;-)
>
>> Antoine Pelisse (5):
>>   Use split_ident_line to parse author and committer
>>   mailmap: Remove buffer length limit in map_user
>>   mailmap: Add mailmap structure to rev_info and pp
>>   pretty: Use mailmap to display username and email
>>   log: Add --use-mailmap option
>>
>>  builtin/blame.c | 59 +++++++++++++++++-------------------------------
>>  builtin/log.c   |  9 +++++++-
>>  commit.h        |  1 +
>>  log-tree.c      |  1 +
>>  mailmap.c       | 16 ++++++-------
>>  pretty.c        | 70 ++++++++++++++++++++++++++++++++++++---------------------
>>  revision.h      |  1 +
>>  7 files changed, 84 insertions(+), 73 deletions(-)
>>
>> --
>> 1.8.1.rc1.5.g7e0651a

^ permalink raw reply

* Re: [PATCH 0/5] Allow git log to use mailmap file
From: Junio C Hamano @ 2012-12-11 22:33 UTC (permalink / raw)
  To: Antoine Pelisse; +Cc: git, Rich Midwinter
In-Reply-To: <1355264493-8298-1-git-send-email-apelisse@gmail.com>

Antoine Pelisse <apelisse@gmail.com> writes:

> Implement the feature suggested here [1] by Rich Mindwinter
> and Junio C Hamano (and following his advices)
>
> This is a pre-version so there are a bunch of things still missing,
> among them:
>  - There is no tests
>  - Grep search for mailmap author/committer is not available
>  - There is no documentation of the new option
>
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/211270

That was quick ;-)

> Antoine Pelisse (5):
>   Use split_ident_line to parse author and committer
>   mailmap: Remove buffer length limit in map_user
>   mailmap: Add mailmap structure to rev_info and pp
>   pretty: Use mailmap to display username and email
>   log: Add --use-mailmap option
>
>  builtin/blame.c | 59 +++++++++++++++++-------------------------------
>  builtin/log.c   |  9 +++++++-
>  commit.h        |  1 +
>  log-tree.c      |  1 +
>  mailmap.c       | 16 ++++++-------
>  pretty.c        | 70 ++++++++++++++++++++++++++++++++++++---------------------
>  revision.h      |  1 +
>  7 files changed, 84 insertions(+), 73 deletions(-)
>
> --
> 1.8.1.rc1.5.g7e0651a

^ permalink raw reply

* [PATCH] fetch: ignore wildcarded refspecs that update local symbolic refs
From: Junio C Hamano @ 2012-12-11 22:32 UTC (permalink / raw)
  To: git; +Cc: Shawn O . Pearce, Jay Soffian, Stefan Zager
In-Reply-To: <7v62488j8a.fsf_-_@alter.siamese.dyndns.org>

In a repository cloned from somewhere else, you typically have a
symbolic ref refs/remotes/origin/HEAD pointing at the 'master'
remote-tracking ref that is next to it.  When fetching into such a
repository with "git fetch --mirror" from another repository that
was similarly cloned, the implied wildcard refspec refs/*:refs/*
will end up asking to update refs/remotes/origin/HEAD with the
object at refs/remotes/origin/HEAD at the remote side, while asking
to update refs/remotes/origin/master the same way.  Depending on the
order the two updates happen, the latter one would find that the
value of the ref before it is updated has changed from what the code
expects.

When the user asks to update the underlying ref via the symbolic ref
explicitly without using a wildcard refspec, e.g. "git fetch $there
refs/heads/master:refs/remotes/origin/HEAD", we should still let him
do so, but when expanding wildcard refs, it will result in a more
intuitive outcome if we simply ignore local symbolic refs.

As the purpose of the symbolic ref refs/remotes/origin/HEAD is to
follow the ref it points at (e.g. refs/remotes/origin/master), its
value would change when the underlying ref is updated.

Earlier commit da3efdb (receive-pack: detect aliased updates which
can occur with symrefs, 2010-04-19) fixed a similar issue for "git
push".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This time with minimal tests and an updated log message.

 remote.c                     | 13 ++++++++++++-
 t/t5535-fetch-push-symref.sh | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)
 create mode 100755 t/t5535-fetch-push-symref.sh

diff --git a/remote.c b/remote.c
index 04fd9ea..a72748c 100644
--- a/remote.c
+++ b/remote.c
@@ -1370,6 +1370,16 @@ int branch_merge_matches(struct branch *branch,
 	return refname_match(branch->merge[i]->src, refname, ref_fetch_rules);
 }
 
+static int ignore_symref_update(const char *refname)
+{
+	unsigned char sha1[20];
+	int flag;
+
+	if (!resolve_ref_unsafe(refname, sha1, 0, &flag))
+		return 0; /* non-existing refs are OK */
+	return (flag & REF_ISSYMREF);
+}
+
 static struct ref *get_expanded_map(const struct ref *remote_refs,
 				    const struct refspec *refspec)
 {
@@ -1383,7 +1393,8 @@ static struct ref *get_expanded_map(const struct ref *remote_refs,
 		if (strchr(ref->name, '^'))
 			continue; /* a dereference item */
 		if (match_name_with_pattern(refspec->src, ref->name,
-					    refspec->dst, &expn_name)) {
+					    refspec->dst, &expn_name) &&
+		    !ignore_symref_update(expn_name)) {
 			struct ref *cpy = copy_ref(ref);
 
 			cpy->peer_ref = alloc_ref(expn_name);
diff --git a/t/t5535-fetch-push-symref.sh b/t/t5535-fetch-push-symref.sh
new file mode 100755
index 0000000..8ed58d2
--- /dev/null
+++ b/t/t5535-fetch-push-symref.sh
@@ -0,0 +1,42 @@
+#!/bin/sh
+
+test_description='avoiding conflicting update thru symref aliasing'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	test_commit one &&
+	git clone . src &&
+	git clone src dst1 &&
+	git clone src dst2 &&
+	test_commit two &&
+	( cd src && git pull )
+'
+
+test_expect_success 'push' '
+	(
+		cd src &&
+		git push ../dst1 "refs/remotes/*:refs/remotes/*"
+	) &&
+	git ls-remote src "refs/remotes/*" >expect &&
+	git ls-remote dst1 "refs/remotes/*" >actual &&
+	test_cmp expect actual &&
+	( cd src && git symbolic-ref refs/remotes/origin/HEAD ) >expect &&
+	( cd dst1 && git symbolic-ref refs/remotes/origin/HEAD ) >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'fetch' '
+	(
+		cd dst2 &&
+		git fetch ../src "refs/remotes/*:refs/remotes/*"
+	) &&
+	git ls-remote src "refs/remotes/*" >expect &&
+	git ls-remote dst2 "refs/remotes/*" >actual &&
+	test_cmp expect actual &&
+	( cd src && git symbolic-ref refs/remotes/origin/HEAD ) >expect &&
+	( cd dst2 && git symbolic-ref refs/remotes/origin/HEAD ) >actual &&
+	test_cmp expect actual
+'
+
+test_done
-- 
1.8.1.rc1.128.gd8d1528

^ permalink raw reply related

* Re: (bug?) Inconsistent workdir file timestamps after initial clone.
From: Junio C Hamano @ 2012-12-11 22:30 UTC (permalink / raw)
  To: Marc Branchaud; +Cc: Git Mailing List
In-Reply-To: <50C7AE84.2060400@xiplink.com>

Marc Branchaud <marcnarc@xiplink.com> writes:

> My point is that the initial checkout into an empty working directory should
> create all files with the same timestamp.
>
> Or, to be a bit more precise, whenever git-checkout *creates* files in the
> work dir, *all* the created files should have the *same* timestamp (i.e. the
> current time measured at the start of the checkout's execution, not some
> bizarro other time specified by some arcane heuristic).

My knee-jerk reaction is that it is insane to do so, but what other
SCM does such a thing? Even "tar xf" wouldn't do that, I think.

>> While not including files that can be rebuilt from the source may be
>> the ideal solution, I've seen projects hide rules to rebuild such a
>> "generated but needs special tools to build" and/or a "generated but
>> normal developers do not have any business rebuilding" file (in your
>> case, Makefile.in) in their Makefiles from the normal targets (like
>> "make all") for this exact reason, when they choose to distribute
>> such files by including in their commits.
>
> I prefer to use the third-party code as-is, without hacking it, to have
> smooth upgrades in the future.

Then perhaps take the complaints to that third-party upstream, not
here?

^ permalink raw reply

* [PATCH 5/5] log: Add --use-mailmap option
From: Antoine Pelisse @ 2012-12-11 22:21 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Rich Midwinter, Antoine Pelisse
In-Reply-To: <1355264493-8298-1-git-send-email-apelisse@gmail.com>

Add the --use-mailmap option to log commands. It allows
to display names from mailmap file when displaying logs,
whatever the format used.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 builtin/log.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/builtin/log.c b/builtin/log.c
index e7b7db1..d2bd8ce 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -22,6 +22,7 @@
 #include "branch.h"
 #include "streaming.h"
 #include "version.h"
+#include "mailmap.h"
 
 /* Set a default date-time format for git log ("log.date" config variable) */
 static const char *default_date_mode = NULL;
@@ -94,11 +95,12 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 			 struct rev_info *rev, struct setup_revision_opt *opt)
 {
 	struct userformat_want w;
-	int quiet = 0, source = 0;
+	int quiet = 0, source = 0, mailmap = 0;
 
 	const struct option builtin_log_options[] = {
 		OPT_BOOLEAN(0, "quiet", &quiet, N_("suppress diff output")),
 		OPT_BOOLEAN(0, "source", &source, N_("show source")),
+		OPT_BOOLEAN(0, "use-mailmap", &mailmap, N_("Use mail map file")),
 		{ OPTION_CALLBACK, 0, "decorate", NULL, NULL, N_("decorate options"),
 		  PARSE_OPT_OPTARG, decorate_callback},
 		OPT_END()
@@ -136,6 +138,11 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 	if (source)
 		rev->show_source = 1;
 
+	if (mailmap) {
+		rev->mailmap = xcalloc(1, sizeof(struct string_list));
+		read_mailmap(rev->mailmap, NULL);
+	}
+
 	if (rev->pretty_given && rev->commit_format == CMIT_FMT_RAW) {
 		/*
 		 * "log --pretty=raw" is special; ignore UI oriented
-- 
1.8.1.rc1.5.g7e0651a

^ permalink raw reply related

* [PATCH 4/5] pretty: Use mailmap to display username and email
From: Antoine Pelisse @ 2012-12-11 22:21 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Rich Midwinter, Antoine Pelisse
In-Reply-To: <1355264493-8298-1-git-send-email-apelisse@gmail.com>

Use the mailmap information to display the correct
username and email address in all log commands.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 pretty.c | 46 +++++++++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 15 deletions(-)

diff --git a/pretty.c b/pretty.c
index 6730add..e232aaa 100644
--- a/pretty.c
+++ b/pretty.c
@@ -387,6 +387,8 @@ void pp_user_info(const struct pretty_print_context *pp,
 		  const char *what, struct strbuf *sb,
 		  const char *line, const char *encoding)
 {
+	char person_name[1024];
+	char person_mail[1024];
 	struct ident_split ident;
 	int linelen, namelen;
 	char *line_end, *date;
@@ -405,41 +407,55 @@ void pp_user_info(const struct pretty_print_context *pp,
 	if (split_ident_line(&ident, line, linelen))
 		return;
 
-	namelen = ident.mail_end - ident.name_begin + 1;
+	memcpy(person_mail, ident.mail_begin, ident.mail_end - ident.mail_begin);
+	person_mail[ident.mail_end - ident.mail_begin] = 0;
+
+	memcpy(person_name, ident.name_begin, ident.name_end - ident.name_begin);
+	person_name[ident.name_end - ident.name_begin] = 0;
+
+	if (pp->mailmap)
+		map_user(pp->mailmap, person_mail, sizeof(person_mail),
+			 person_name, sizeof(person_name));
+
+	namelen = strlen(person_name) + strlen(person_mail);
 	time = strtoul(ident.date_begin, &date, 10);
 	tz = strtol(date, NULL, 10);
 
 	if (pp->fmt == CMIT_FMT_EMAIL) {
 		int display_name_length;
 
-		display_name_length = ident.name_end - ident.name_begin;
+		display_name_length = strlen(person_name);
 
 		strbuf_addstr(sb, "From: ");
-		if (needs_rfc2047_encoding(line, display_name_length, RFC2047_ADDRESS)) {
-			add_rfc2047(sb, line, display_name_length,
+		if (needs_rfc2047_encoding(person_name, display_name_length, RFC2047_ADDRESS)) {
+			add_rfc2047(sb, person_name, display_name_length,
 						encoding, RFC2047_ADDRESS);
 			max_length = 76; /* per rfc2047 */
-		} else if (needs_rfc822_quoting(line, display_name_length)) {
+		} else if (needs_rfc822_quoting(person_name,
+						display_name_length)) {
 			struct strbuf quoted = STRBUF_INIT;
-			add_rfc822_quoted(&quoted, line, display_name_length);
+			add_rfc822_quoted(&quoted, person_name,
+					  display_name_length);
 			strbuf_add_wrapped_bytes(sb, quoted.buf, quoted.len,
 							-6, 1, max_length);
 			strbuf_release(&quoted);
 		} else {
-			strbuf_add_wrapped_bytes(sb, line, display_name_length,
-							-6, 1, max_length);
+			strbuf_add_wrapped_bytes(sb, person_name,
+						 display_name_length,
+						 -6, 1, max_length);
 		}
-		if (namelen - display_name_length + last_line_length(sb) > max_length) {
+		if (namelen - display_name_length + last_line_length(sb) > max_length)
 			strbuf_addch(sb, '\n');
-			if (!isspace(ident.name_end[0]))
-				strbuf_addch(sb, ' ');
-		}
-		strbuf_add(sb, ident.name_end, namelen - display_name_length);
+
+		strbuf_addch(sb, ' ');
+		strbuf_addch(sb, '<');
+		strbuf_add(sb, person_mail, strlen(person_mail));
+		strbuf_addch(sb, '>');
 		strbuf_addch(sb, '\n');
 	} else {
-		strbuf_addf(sb, "%s: %.*s%.*s\n", what,
+		strbuf_addf(sb, "%s: %.*s%s <%s>\n", what,
 			      (pp->fmt == CMIT_FMT_FULLER) ? 4 : 0,
-			      "    ", namelen, line);
+			    "    ", person_name, person_mail);
 	}
 	switch (pp->fmt) {
 	case CMIT_FMT_MEDIUM:
-- 
1.8.1.rc1.5.g7e0651a

^ permalink raw reply related

* [PATCH 3/5] mailmap: Add mailmap structure to rev_info and pp
From: Antoine Pelisse @ 2012-12-11 22:21 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Rich Midwinter, Antoine Pelisse
In-Reply-To: <1355264493-8298-1-git-send-email-apelisse@gmail.com>

the mailmap string_list structure filled with mailmap
information is passed along from rev_info to pretty_print_context
to provide mailmap information to pretty print each commits
with the correct username and email.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 commit.h   | 1 +
 log-tree.c | 1 +
 revision.h | 1 +
 3 files changed, 3 insertions(+)

diff --git a/commit.h b/commit.h
index b6ad8f3..7f8f987 100644
--- a/commit.h
+++ b/commit.h
@@ -89,6 +89,7 @@ struct pretty_print_context {
 	char *notes_message;
 	struct reflog_walk_info *reflog_info;
 	const char *output_encoding;
+	struct string_list *mailmap;
 };
 
 struct userformat_want {
diff --git a/log-tree.c b/log-tree.c
index 4f86def..92254fd 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -671,6 +671,7 @@ void show_log(struct rev_info *opt)
 	ctx.preserve_subject = opt->preserve_subject;
 	ctx.reflog_info = opt->reflog_info;
 	ctx.fmt = opt->commit_format;
+	ctx.mailmap = opt->mailmap;
 	pretty_print_commit(&ctx, commit, &msgbuf);
 
 	if (opt->add_signoff)
diff --git a/revision.h b/revision.h
index 059bfff..83a79f5 100644
--- a/revision.h
+++ b/revision.h
@@ -143,6 +143,7 @@ struct rev_info {
 	const char	*subject_prefix;
 	int		no_inline;
 	int		show_log_size;
+	struct string_list *mailmap;
 
 	/* Filter by commit log message */
 	struct grep_opt	grep_filter;
-- 
1.8.1.rc1.5.g7e0651a

^ permalink raw reply related

* [PATCH 1/5] Use split_ident_line to parse author and committer
From: Antoine Pelisse @ 2012-12-11 22:21 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Rich Midwinter, Antoine Pelisse
In-Reply-To: <1355264493-8298-1-git-send-email-apelisse@gmail.com>

Currently blame.c::get_acline and pretty.c::pp_user_info()
are parsing author name, email, time and tz themselves.

Use ident.c::split_ident_line for better code reuse.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 builtin/blame.c | 59 ++++++++++++++++++++-------------------------------------
 pretty.c        | 32 +++++++++++++++++--------------
 2 files changed, 39 insertions(+), 52 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index cfae569..dd4aff9 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1343,8 +1343,9 @@ static void get_ac_line(const char *inbuf, const char *what,
 			int mail_len, char *mail,
 			unsigned long *time, const char **tz)
 {
-	int len, tzlen, maillen;
-	char *tmp, *endp, *timepos, *mailpos;
+	struct ident_split ident;
+	int len, tzlen, maillen, namelen;
+	char *tmp, *endp, *mailpos;
 
 	tmp = strstr(inbuf, what);
 	if (!tmp)
@@ -1355,7 +1356,10 @@ static void get_ac_line(const char *inbuf, const char *what,
 		len = strlen(tmp);
 	else
 		len = endp - tmp;
-	if (person_len <= len) {
+	if (person_len <= len)
+		goto error_out;
+
+	if (split_ident_line(&ident, tmp, len)) {
 	error_out:
 		/* Ugh */
 		*tz = "(unknown)";
@@ -1364,47 +1368,26 @@ static void get_ac_line(const char *inbuf, const char *what,
 		*time = 0;
 		return;
 	}
-	memcpy(person, tmp, len);
 
-	tmp = person;
-	tmp += len;
-	*tmp = 0;
-	while (person < tmp && *tmp != ' ')
-		tmp--;
-	if (tmp <= person)
-		goto error_out;
-	*tz = tmp+1;
-	tzlen = (person+len)-(tmp+1);
+	namelen = ident.name_end - ident.name_begin;
+	memcpy(person, ident.name_begin, namelen);
+	person[namelen] = 0;
 
-	*tmp = 0;
-	while (person < tmp && *tmp != ' ')
-		tmp--;
-	if (tmp <= person)
-		goto error_out;
-	*time = strtoul(tmp, NULL, 10);
-	timepos = tmp;
+	maillen = ident.mail_end - ident.mail_begin + 2;
+	memcpy(mail, ident.mail_begin - 1, maillen);
+	mail[maillen] = 0;
 
-	*tmp = 0;
-	while (person < tmp && !(*tmp == ' ' && tmp[1] == '<'))
-		tmp--;
-	if (tmp <= person)
-		return;
-	mailpos = tmp + 1;
-	*tmp = 0;
-	maillen = timepos - tmp;
-	memcpy(mail, mailpos, maillen);
+	*time = strtoul(ident.date_begin, NULL, 10);
 
-	if (!mailmap.nr)
-		return;
+	tzlen = ident.tz_end - ident.tz_begin;
 
-	/*
-	 * mailmap expansion may make the name longer.
-	 * make room by pushing stuff down.
-	 */
-	tmp = person + person_len - (tzlen + 1);
-	memmove(tmp, *tz, tzlen);
+	/* Place tz at the end of person */
+	*tz = tmp = person + person_len - (tzlen + 1);
+	memcpy(tmp, ident.tz_begin, tzlen);
 	tmp[tzlen] = 0;
-	*tz = tmp;
+
+	if (!mailmap.nr)
+		return;
 
 	/*
 	 * Now, convert both name and e-mail using mailmap
diff --git a/pretty.c b/pretty.c
index 5bdc2e7..6730add 100644
--- a/pretty.c
+++ b/pretty.c
@@ -387,29 +387,33 @@ void pp_user_info(const struct pretty_print_context *pp,
 		  const char *what, struct strbuf *sb,
 		  const char *line, const char *encoding)
 {
+	struct ident_split ident;
+	int linelen, namelen;
+	char *line_end, *date;
 	int max_length = 78; /* per rfc2822 */
-	char *date;
-	int namelen;
 	unsigned long time;
 	int tz;
 
 	if (pp->fmt == CMIT_FMT_ONELINE)
 		return;
-	date = strchr(line, '>');
-	if (!date)
+
+	line_end = strchr(line, '\n');
+	if (!line_end)
+		return;
+
+	linelen = ++line_end - line;
+	if (split_ident_line(&ident, line, linelen))
 		return;
-	namelen = ++date - line;
-	time = strtoul(date, &date, 10);
+
+	namelen = ident.mail_end - ident.name_begin + 1;
+	time = strtoul(ident.date_begin, &date, 10);
 	tz = strtol(date, NULL, 10);
 
 	if (pp->fmt == CMIT_FMT_EMAIL) {
-		char *name_tail = strchr(line, '<');
 		int display_name_length;
-		if (!name_tail)
-			return;
-		while (line < name_tail && isspace(name_tail[-1]))
-			name_tail--;
-		display_name_length = name_tail - line;
+
+		display_name_length = ident.name_end - ident.name_begin;
+
 		strbuf_addstr(sb, "From: ");
 		if (needs_rfc2047_encoding(line, display_name_length, RFC2047_ADDRESS)) {
 			add_rfc2047(sb, line, display_name_length,
@@ -427,10 +431,10 @@ void pp_user_info(const struct pretty_print_context *pp,
 		}
 		if (namelen - display_name_length + last_line_length(sb) > max_length) {
 			strbuf_addch(sb, '\n');
-			if (!isspace(name_tail[0]))
+			if (!isspace(ident.name_end[0]))
 				strbuf_addch(sb, ' ');
 		}
-		strbuf_add(sb, name_tail, namelen - display_name_length);
+		strbuf_add(sb, ident.name_end, namelen - display_name_length);
 		strbuf_addch(sb, '\n');
 	} else {
 		strbuf_addf(sb, "%s: %.*s%.*s\n", what,
-- 
1.8.1.rc1.5.g7e0651a

^ permalink raw reply related

* [PATCH 2/5] mailmap: Remove buffer length limit in map_user
From: Antoine Pelisse @ 2012-12-11 22:21 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Rich Midwinter, Antoine Pelisse
In-Reply-To: <1355264493-8298-1-git-send-email-apelisse@gmail.com>

Remove the hard limit set by mail buffer in map_user and
use the strbuf API to replace it.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 mailmap.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/mailmap.c b/mailmap.c
index ea4b471..e636278 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -193,7 +193,8 @@ int map_user(struct string_list *map,
 	char *end_of_email;
 	struct string_list_item *item;
 	struct mailmap_entry *me;
-	char buf[1024], *mailbuf;
+	struct strbuf buf = STRBUF_INIT;
+	char *mailbuf;
 	int i;
 
 	/* figure out space requirement for email */
@@ -204,15 +205,13 @@ int map_user(struct string_list *map,
 		if (!end_of_email)
 			return 0;
 	}
-	if (end_of_email - email + 1 < sizeof(buf))
-		mailbuf = buf;
-	else
-		mailbuf = xmalloc(end_of_email - email + 1);
 
 	/* downcase the email address */
+	strbuf_grow(&buf, end_of_email - email);
 	for (i = 0; i < end_of_email - email; i++)
-		mailbuf[i] = tolower(email[i]);
-	mailbuf[i] = 0;
+		strbuf_addch(&buf, tolower(email[i]));
+	strbuf_addch(&buf, 0);
+	mailbuf = strbuf_detach(&buf, 0);
 
 	debug_mm("map_user: map '%s' <%s>\n", name, mailbuf);
 	item = string_list_lookup(map, mailbuf);
@@ -226,8 +225,7 @@ int map_user(struct string_list *map,
 				item = subitem;
 		}
 	}
-	if (mailbuf != buf)
-		free(mailbuf);
+	free(mailbuf);
 	if (item != NULL) {
 		struct mailmap_info *mi = (struct mailmap_info *)item->util;
 		if (mi->name == NULL && (mi->email == NULL || maxlen_email == 0)) {
-- 
1.8.1.rc1.5.g7e0651a

^ permalink raw reply related

* [PATCH 0/5] Allow git log to use mailmap file
From: Antoine Pelisse @ 2012-12-11 22:21 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Rich Midwinter, Antoine Pelisse

Implement the feature suggested here [1] by Rich Mindwinter
and Junio C Hamano (and following his advices)

This is a pre-version so there are a bunch of things still missing,
among them:
 - There is no tests
 - Grep search for mailmap author/committer is not available
 - There is no documentation of the new option

[1]: http://thread.gmane.org/gmane.comp.version-control.git/211270

Antoine Pelisse (5):
  Use split_ident_line to parse author and committer
  mailmap: Remove buffer length limit in map_user
  mailmap: Add mailmap structure to rev_info and pp
  pretty: Use mailmap to display username and email
  log: Add --use-mailmap option

 builtin/blame.c | 59 +++++++++++++++++-------------------------------
 builtin/log.c   |  9 +++++++-
 commit.h        |  1 +
 log-tree.c      |  1 +
 mailmap.c       | 16 ++++++-------
 pretty.c        | 70 ++++++++++++++++++++++++++++++++++++---------------------
 revision.h      |  1 +
 7 files changed, 84 insertions(+), 73 deletions(-)

--
1.8.1.rc1.5.g7e0651a

^ permalink raw reply


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