Incorrect pipe for git log graph when using --name-status option

Incorrect pipe for git log graph when using --name-status option

[hIpPy]

The `git log` command with `graph` and pretty format works correctly
as expected.

$ git log --graph --pretty=format:'%h' -2
* 714a14e
* 87dce5f


However, with `--name-status` option added, there is a pipe
incorrectly placed after the commit hash (example below).

$ git log --graph --pretty=format:'%h' -2 --name-status
* 714a14e|
| M README.md
| A rm.Extensions/BitSet.cs
| M rm.Extensions/Properties/AssemblyInfo.cs
| M rm.Extensions/rm.Extensions.csproj
| A rm.ExtensionsTest/BitSetTest.cs
| M rm.ExtensionsTest/rm.ExtensionsTest.csproj

* 87dce5f|
| M rm.Extensions/GraphExtension.cs
| M rm.Extensions/Wrapped.cs
| M rm.Extensions/WrappedExtension.cs
| M rm.Extensions/rm.Extensions.csproj


IMHO, I think this is a bug. I think the correct output should be
below.

$ git log --graph --pretty=format:'%h' -2 --name-status
* 714a14e
| M README.md
| A rm.Extensions/BitSet.cs
| M rm.Extensions/Properties/AssemblyInfo.cs
| M rm.Extensions/rm.Extensions.csproj

| A rm.ExtensionsTest/BitSetTest.cs
| M rm.ExtensionsTest/rm.ExtensionsTest.csproj
|
* 87dce5f
| M rm.Extensions/GraphExtension.cs
| M rm.Extensions/Wrapped.cs
| M rm.Extensions/WrappedExtension.cs

| M rm.Extensions/rm.Extensions.csproj

I'm using this:

git version 2.11.0.windows.1
GNU bash, version 4.3.46(2)-release (x86_64-pc-msys)
Windows 8.1 64-bit


Thanks,
RM

Re: Incorrect pipe for git log graph when using --name-status option

[Junio C Hamano]

hIpPy <hippy2981@gmail.com> writes:

> The `git log` command with `graph` and pretty format works correctly
> as expected.
>
> $ git log --graph --pretty=format:'%h' -2
> * 714a14e
> * 87dce5f
>
>
> However, with `--name-status` option added, there is a pipe
> incorrectly placed after the commit hash (example below).
>
> $ git log --graph --pretty=format:'%h' -2 --name-status
> * 714a14e|
> | M README.md

Is it a --name-status, or is it your own custom format, that is
causing the above issue?

 - What happens if you stop using --pretty=format:%h and replace it
   with something like --oneline?

 - What happens if you keep using --pretty=format:%h but replace
   --name-status with something else, e.g. --raw or --stat?

Re: Incorrect pipe for git log graph when using --name-status option

[hIpPy]

On Mon, Feb 13, 2017 at 11:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
> hIpPy <hippy2981@gmail.com> writes:
>
>> The `git log` command with `graph` and pretty format works correctly
>> as expected.
>>
>> $ git log --graph --pretty=format:'%h' -2
>> * 714a14e
>> * 87dce5f
>>
>>
>> However, with `--name-status` option added, there is a pipe
>> incorrectly placed after the commit hash (example below).
>>
>> $ git log --graph --pretty=format:'%h' -2 --name-status
>> * 714a14e|
>> | M README.md
>
> Is it a --name-status, or is it your own custom format, that is
> causing the above issue?
>
>  - What happens if you stop using --pretty=format:%h and replace it
>    with something like --oneline?
>
>  - What happens if you keep using --pretty=format:%h but replace
>    --name-status with something else, e.g. --raw or --stat?
>



 - What happens if you stop using --pretty=format:%h and replace it
   with something like --oneline?
--oneline works correctly as expected (example below).

$ git log --graph --oneline -2 --name-status
* bf7ace7daf (HEAD -> rm/option-for-name-status) wip: --ns for --name-status
| M diff.c
*   592e5c5bce (origin/master, origin/HEAD, master) Merge pull request
#994 from jeffhostetler/jeffhostetler/fscache_nfd
|\


 - What happens if you keep using --pretty=format:%h but replace
   --name-status with something else, e.g. --raw or --stat?
I see the same issue with --raw and --stat (examples below).

$ git log --graph --pretty=format:'%h' -2 --raw
* bf7ace7daf|
| :100644 100644 84dba60c40... 87dfabf4a9... M  diff.c

*   592e5c5bce
|\

$ git log --graph --pretty=format:'%h' -2 --stat
* bf7ace7daf|
|  diff.c | 2 +-
|  1 file changed, 1 insertion(+), 1 deletion(-)

*   592e5c5bce
|\

I believe it's not my custom format that is causing the issue.
I'm including this information that may not be relevant. I apologize
in advance for that. I had simplified the custom format in my
previous email. For below custom format I still see the pipe
incorrectly placed.

$ git log --graph --date=relative --decorate --abbrev-commit
--format=format:'%h - %aD (%ar)%d%n     %s - %an' -2 --name-status
* bf7ace7daf - Mon, 6 Feb 2017 13:09:32 -0800 (7 days ago) (HEAD ->
rm/option-for-name-status)
|      wip: --ns for --name-status - Rishikesh Mandvikar|
| M diff.c

*   592e5c5bce - Wed, 1 Feb 2017 16:35:30 +0100 (12 days ago)
(origin/master, origin/HEAD, master)
|\       Merge pull request #994 from
jeffhostetler/jeffhostetler/fscache_nfd - Johannes Schindelin


If I were to put a '%n' at the end (example below), the pipe is
correctly placed with or without the --name-status option. But in
case of "without the --name-status option", there is an extra blank
line. It seems that my custom format is requiring a %n at the end. I
consider my custom format as a --twoline option and I feel the
behavior should match --oneline when using options.

With --name-status: This works correctly.

$ git log --graph --date=relative --decorate --abbrev-commit
--format=format:'%h - %aD (%ar)%d%n     %s - %an%n' -2 --name-status
* bf7ace7daf - Mon, 6 Feb 2017 13:09:32 -0800 (7 days ago) (HEAD ->
rm/option-for-name-status)
|      wip: --ns for --name-status - Rishikesh Mandvikar
|
| M diff.c
|
*   592e5c5bce - Wed, 1 Feb 2017 16:35:30 +0100 (12 days ago)
(origin/master, origin/HEAD, master)
|\       Merge pull request #994 from
jeffhostetler/jeffhostetler/fscache_nfd - Johannes Schindelin

Without --name-status: This works but has extra blank line between
commits though.

$ git log --graph --date=relative --decorate --abbrev-commit
--format=format:'%h - %aD (%ar)%d%n     %s - %an%n' -2
* bf7ace7daf - Mon, 6 Feb 2017 13:09:32 -0800 (7 days ago) (HEAD ->
rm/option-for-name-status)
|      wip: --ns for --name-status - Rishikesh Mandvikar
|
*   592e5c5bce - Wed, 1 Feb 2017 16:35:30 +0100 (12 days ago)
(origin/master, origin/HEAD, master)
|\       Merge pull request #994 from
jeffhostetler/jeffhostetler/fscache_nfd - Johannes Schindelin

I think that requiring to end custom formats with %n with options
is not good. So I think this is a bug.

Thanks,
RM

Re: Incorrect pipe for git log graph when using --name-status option

[Jeff King]

On Mon, Feb 13, 2017 at 12:55:07PM -0800, hIpPy wrote:

> I think that requiring to end custom formats with %n with options
> is not good. So I think this is a bug.

Yes, you don't normally need to do that.

I think the problem is specifically related to the "terminator" versus
"separator" semantics. Try:

  git log --graph --name-status --pretty=format:%h

versus:

  git log --graph --name-status --pretty=tformat:%h

The latter works fine. The problem seems to happen with all diff
formats, so I think the issue is that git is not aggressive enough about
inserting the newline at the right spot when using separator mode.

-Peff

Re: Incorrect pipe for git log graph when using --name-status option

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I think the problem is specifically related to the "terminator" versus
> "separator" semantics. Try:
>
>   git log --graph --name-status --pretty=format:%h
>
> versus:
>
>   git log --graph --name-status --pretty=tformat:%h
>
> The latter works fine. The problem seems to happen with all diff
> formats, so I think the issue is that git is not aggressive enough about
> inserting the newline at the right spot when using separator mode.

I guess that is one of the reasons why we made --format=%h a synonym
to --pretty=tformat:%h and not --pretty=format:%h ;-)

Re: Incorrect pipe for git log graph when using --name-status option

[Jeff King]

On Mon, Feb 13, 2017 at 01:18:40PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I think the problem is specifically related to the "terminator" versus
> > "separator" semantics. Try:
> >
> >   git log --graph --name-status --pretty=format:%h
> >
> > versus:
> >
> >   git log --graph --name-status --pretty=tformat:%h
> >
> > The latter works fine. The problem seems to happen with all diff
> > formats, so I think the issue is that git is not aggressive enough about
> > inserting the newline at the right spot when using separator mode.
> 
> I guess that is one of the reasons why we made --format=%h a synonym
> to --pretty=tformat:%h and not --pretty=format:%h ;-)

Yeah. I have never found "--pretty=format" to do what I want versus
tformat. I wish we could just get rid of it, but I think it is likely to
be used as a plumbing interface.

So I think there probably _is_ a bug in it to be fixed, but my immediate
response is "don't ever use --pretty=format:".

-Peff

Re: Incorrect pipe for git log graph when using --name-status option

[hIpPy]

On Mon, Feb 13, 2017 at 1:27 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Feb 13, 2017 at 01:18:40PM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>> > I think the problem is specifically related to the "terminator" versus
>> > "separator" semantics. Try:
>> >
>> >   git log --graph --name-status --pretty=format:%h
>> >
>> > versus:
>> >
>> >   git log --graph --name-status --pretty=tformat:%h
>> >
>> > The latter works fine. The problem seems to happen with all diff
>> > formats, so I think the issue is that git is not aggressive enough about
>> > inserting the newline at the right spot when using separator mode.
>>
>> I guess that is one of the reasons why we made --format=%h a synonym
>> to --pretty=tformat:%h and not --pretty=format:%h ;-)
>
> Yeah. I have never found "--pretty=format" to do what I want versus
> tformat. I wish we could just get rid of it, but I think it is likely to
> be used as a plumbing interface.
>
> So I think there probably _is_ a bug in it to be fixed, but my immediate
> response is "don't ever use --pretty=format:".
>
> -Peff

I have 1 issue with tformat in that I feel it reduces
readability a bit when using options. But I can use it if that
is recommended over the other.

Without --name-status: This is OK.

$ git log --graph --date=relative --decorate --abbrev-commit
--format='%h - %aD (%ar)%d%n     %s - %an' -2
* bf7ace7daf - Mon, 6 Feb 2017 13:09:32 -0800 (7 days ago) (HEAD ->
rm/option-for-name-status)
|      wip: --ns for --name-status - Rishikesh Mandvikar
*   592e5c5bce - Wed, 1 Feb 2017 16:35:30 +0100 (12 days ago)
(origin/master, origin/HEAD, master)
|\       Merge pull request #994 from
jeffhostetler/jeffhostetler/fscache_nfd - Johannes Schindelin


With --name-status: I'm sorry if I nitpick here but I think the
--name-status items should either be preceeded and followed by
blank line OR not (as in oneline) but not just preceded (example
below).

$ git log --graph --date=relative --decorate --abbrev-commit
--format='%h - %aD (%ar)%d%n     %s - %an' -2 --name-status
* bf7ace7daf - Mon, 6 Feb 2017 13:09:32 -0800 (7 days ago) (HEAD ->
rm/option-for-name-status)
|      wip: --ns for --name-status - Rishikesh Mandvikar
|
| M diff.c
*   592e5c5bce - Wed, 1 Feb 2017 16:35:30 +0100 (12 days ago)
(origin/master, origin/HEAD, master)
|\       Merge pull request #994 from
jeffhostetler/jeffhostetler/fscache_nfd - Johannes Schindelin


So either of below is better that above (I feel):

Blank lines before and after --name-status items:

$ git log --graph --date=relative --decorate --abbrev-commit
--format='%h - %aD (%ar)%d%n     %s - %an' -2 --name-status
* bf7ace7daf - Mon, 6 Feb 2017 13:09:32 -0800 (7 days ago) (HEAD ->
rm/option-for-name-status)
|      wip: --ns for --name-status - Rishikesh Mandvikar
|
| M diff.c
|
*   592e5c5bce - Wed, 1 Feb 2017 16:35:30 +0100 (12 days ago)
(origin/master, origin/HEAD, master)
|\       Merge pull request #994 from
jeffhostetler/jeffhostetler/fscache_nfd - Johannes Schindelin

No blank lines before and after --name-status items:

$ git log --graph --date=relative --decorate --abbrev-commit
--format='%h - %aD (%ar)%d%n     %s - %an' -2 --name-status
* bf7ace7daf - Mon, 6 Feb 2017 13:09:32 -0800 (7 days ago) (HEAD ->
rm/option-for-name-status)
|      wip: --ns for --name-status - Rishikesh Mandvikar
| M diff.c
*   592e5c5bce - Wed, 1 Feb 2017 16:35:30 +0100 (12 days ago)
(origin/master, origin/HEAD, master)
|\       Merge pull request #994 from
jeffhostetler/jeffhostetler/fscache_nfd - Johannes Schindelin

What do guys think?

If there is a way to get what I want using tformat?


Thanks,
RM

Re: Incorrect pipe for git log graph when using --name-status option

[Jeff King]

On Mon, Feb 13, 2017 at 01:43:45PM -0800, hIpPy wrote:

> With --name-status: I'm sorry if I nitpick here but I think the
> --name-status items should either be preceeded and followed by
> blank line OR not (as in oneline) but not just preceded (example
> below).

Yeah, I agree the output is a bit ugly.

I don't think there is a way to do what you want currently. The
--oneline code path has some internal magic that suppresses some of the
newlines.

I think it is a good goal that anything that can be expressed via one of
the regular formats (like --oneline) can be expressed via custom
formats. I don't know offhand how hard it would be to make this
particular case work. If somebody is interested in tackling this, they'd
probably need to dig around in pretty.c to see where the ONELINE code
path diverge from the USER_FORMAT one, and then figure out how a user
can specify that they want the ONELINE-ish semantics for their format.

-Peff