Re: [PATCH 2/4] log and rev-list: Fixed newline termination issues with --graph

Junio C Hamano [off-list ref] writes:

I suspect that --pretty=format: (i.e. userformat) should have a way to
explicitly tell which is wanted.  Perhaps we can keep the separator
semantics not to break existing users, and introduce a dummy expand item
(say, '%_') and when it appears in the pattern it would ask for the
terminator semantics instead?

In any case, I'm happy to see that somebody started looking into this, as
this "separator vs terminator" issue in userformat has been nagging me for
quite a while.  It might be good idea to have the change independently
from the graph extension first and then build the graph stuff on top of
the solidified base.  I dunno...

Some alternatives to specify terminator semantics I considered are:

 (1) Presence of %_ in "--pretty=format:..." triggers terminator
     semantics and %_ itself interpolates an empty string; otherwise
     separator semantics is used.

 (2) Presence of %n in "--pretty=format:..." means a multi-line output and
     uses separator as before; lack of %n means it is a one-line format
     and uses terminator.

 (3) A new option --pretty=tformat:... (i.e. tformat instead of format)
     means LF (or NUL) is used as terminator instead of separator;

 (4) A new syntax --pretty=format/... (i.e. slash instead of the usual
     colon) means LF (or NUL) is used as terminator instead of separator;

The first one is what I suggested in the message, but it feels somewhat
hacky.  I suspect that the second one would catch 99% of the cases, but it
is DWIM and it is known that DWIM can go wrong.  I favor design along
the lines of (3) or (4), which I think would be much cleaner.

I however do not particularly like either "tformat" which is a non-word,
nor ":" vs "/" whose differences do not intuitively translate to
"separator vs terminator" distinction.

On Mon, Apr 07, 2008 at 01:52:40AM -0700, Junio C Hamano wrote:

Some alternatives to specify terminator semantics I considered are:
[...]
The first one is what I suggested in the message, but it feels somewhat
hacky.  I suspect that the second one would catch 99% of the cases, but it
is DWIM and it is known that DWIM can go wrong.  I favor design along
the lines of (3) or (4), which I think would be much cleaner.

How about:

  (5) There is no automagic terminator or separator for user formats. %n
      translates to a newline. %N translates to a newline, unless this
      is the final record, in which case it translates to the empty
      string.

So:
    # oneline
    git log --pretty=format:'%h %s%n'
    # multiline
    git log --pretty=format:'%h%nSubject: %s%n%N'

The main drawback is that dropping the automatic separator breaks
existing uses. We could work around this by automatically appending '%n'
if there is no use of "%n" or "%N", but I think that DWIM won't work for
formats which want a non-newline record separator. So maybe a %x/%X
string to mean "separator / terminator" instead of building on newline.
If one doesn't exist, we append "%x" to the format (to give oneline
semantics).

-Peff

Junio C Hamano [off-list ref] writes:

Junio C Hamano [off-list ref] writes:

quoted

I suspect that --pretty=format: (i.e. userformat) should have a way to
explicitly tell which is wanted.  Perhaps we can keep the separator
semantics not to break existing users, and introduce a dummy expand item
(say, '%_') and when it appears in the pattern it would ask for the
terminator semantics instead?

In any case, I'm happy to see that somebody started looking into this, as
this "separator vs terminator" issue in userformat has been nagging me for
quite a while.  It might be good idea to have the change independently
from the graph extension first and then build the graph stuff on top of
the solidified base.  I dunno...

Some alternatives to specify terminator semantics I considered are:

 (1) Presence of %_ in "--pretty=format:..." triggers terminator
     semantics and %_ itself interpolates an empty string; otherwise
     separator semantics is used.

Or %_ might interpolate to _single_ separator, swallowing all
separators that follows it (something like collapsing whitespace).
Either that, or %_ interpolate to separator value, and %*_ collapses
separators (terminators).
 
Bit less hacky, bit more geeky.

 (2) Presence of %n in "--pretty=format:..." means a multi-line output and
     uses separator as before; lack of %n means it is a one-line format
     and uses terminator.

I guess that literal newline in format would also mean multi-line
output.  Also '%b' (body) should mean multi-line output.


BTW. rpm uses [% ... ] to iterate over a set of (parallel) arrays
in --queryformat, which is a bit similar to --pretty=format:<fmt>,
e.g. 'rpm -q --queryformat "[%-50{FILENAMES} %10{FILESIZES}\n]'

BTW2. git-for-each-ref uses _different_ kind of format, %(<name>) and
not %<char>.

 (3) A new option --pretty=tformat:... (i.e. tformat instead of format)
     means LF (or NUL) is used as terminator instead of separator;

 (4) A new syntax --pretty=format/... (i.e. slash instead of the usual
     colon) means LF (or NUL) is used as terminator instead of separator;

The first one is what I suggested in the message, but it feels somewhat
hacky.  I suspect that the second one would catch 99% of the cases, but it
is DWIM and it is known that DWIM can go wrong.  I favor design along
the lines of (3) or (4), which I think would be much cleaner.

I however do not particularly like either "tformat" which is a non-word,
nor ":" vs "/" whose differences do not intuitively translate to
"separator vs terminator" distinction.

"|" instead of ":" wouldn't be a good idea?

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Jeff King [off-list ref] writes:

The main drawback is that dropping the automatic separator breaks
existing uses. We could work around this by automatically appending '%n'
if there is no use of "%n" or "%N",...

Yeah, I agree that sounds quite bad, and that is why I favor more explicit
and independent way to choose between separator and terminator (like the
"tformat" thing).

On Mon, Apr 07, 2008 at 10:43:09AM -0700, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

The main drawback is that dropping the automatic separator breaks
existing uses. We could work around this by automatically appending '%n'
if there is no use of "%n" or "%N",...

Yeah, I agree that sounds quite bad, and that is why I favor more explicit
and independent way to choose between separator and terminator (like the
"tformat" thing).

How about an extra option to explicitly toggle the separator on or
off?  For example, how about "--sep=<count>"?  Using --sep=0 would put
no separator between entries.  --sep=N would put N separators between
entries.

If this option isn't explicitly specified, the default should be
--sep=0 for ONELINE, and --sep=1 for everything else.  Using --sep=2
with USERFORMAT would achieve the behavior of getting an extra line of
padding between entries.


This still leaves unresolved the fact that the USERFORMAT output ends
with a missing newline in most cases.  I can see the argument for
doing this when using -z, which is primarily aimed for programmatic
consumption.  However, without -z, I don't think this is the behavior
most users expect.  For example, try running
"git --no-pager log -1 --pretty=format:%H".

If we were starting from scratch, and didn't have to worry about
breaking existing behavior, I would say that USERFORMAT should always
terminate entries in a newline, and then default to --sep=0.  As it
is, it might be easiest just to leave the current behavior, without a
terminating newline.

-- 
Adam Simpkins
adam@adamsimpkins.net