Re: entry terminator/separator behavior in show_log()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 28, 2008, 16:10 UTC
- Message-ID
- <7vtzhmc63w.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <20080428045504.GA24981@adamsimpkins.net>
Adam Simpkins <adam@adamsimpkins.net> writes:
Show 12 quoted lines
> Variables affecting the behavior > -------------------------------- > > - The sep argument to show_log() > > As far as I can tell, this is always "". show_log() is sometimes > invoked with a sep argument of diffopt->msg_sep. However, > "git grep -w msg_sep" shows that msg_sep is never set to anything but > the empty string. > > Perhaps we could just remove this argument, to reduce complexity and > confusion?
Yes. 39bc9a6 (Add msg_sep to diff_options, 2006-06-25) and f005df3 (diff-tree: Use ---\n as a message separator, 2006-06-27) introduced the variable and used it as the mechanism to add "---\n" at the end of the message, which was a mistake, because it treated "---\n" as a part of the message even though it is not --- it is a separator between the message and the diff part if and only if the diff exists.
Later, 3969cf7 (Fix some more diff options changes., 2006-06-27) corrected this and made generation of --- as part of the log-tree, where "the diff for the commit" is generated. The variable is not used anymore and we can safely remove it.
Show 7 quoted lines
> - Whether or not the log entry ends in a terminating newline > > The log entry for CMIT_FMT_ONELINE never ends in a newline. > The log entry for CMIT_FMT_USERFORMAT only ends in a newline if the > user explicitly places a newline at the end of the format. > > For all other formats, the entry always ends in a terminating newline.
Hmm, interesting. This is true even when the original commit log message ends with an incomplete line.
Show 10 quoted lines
> - Whether or not the log message buffer is empty > > If use_terminator is set and the message buffer returned by > pretty_print_commit() is empty, no newline is printed between entries. > > This can happen, for example, if CMIT_FMT_USERFORMAT is used with an > empty format string. Even though pretty_print_commit() returns an > empty buffer, the log entry itself does not necessarily have to be > empty. For example, the log size will be printed if if the --log-size > argument is used.
This could just be a bug. I do not think of a good reason to treat an empty entry any differently from a single line entry that happens to have zero characters on it.
Show 7 quoted lines
> (2) For CMIT_FMT_ONELINE and CMIT_FMT_USERFORMAT: > (2.2) > If use_terminator is not set (--pretty=format): > > diffopt->line_termination is printed before each log entry but the > first. This results in diffopt->line_termination in between each > entry, and not after the final entry.
IOW, "use separators between entries."
Show 7 quoted lines
> - Case 2.1.2 seems reasonable, but not really necessary. It might be > nicer to simply always print a terminating newline even if the message > buffer is empty. This would appear as an extra blank line after each > entry. > > The --pretty=tformat option is quite new, so I doubt that many people > would complain if we changed this behavior.
Concurred.
> - Case 2.2.1 makes perfect sense. There are probably lots of scripts > out there relying on this behavior.
I would agree with the second sentence, but not necessarily with the first. For script consumption, terminator is easier to deal with than separator if the script is streaming; separator is easier if the script gobbles up everything and then uses split(/$separator/, $everything). They both have their uses.
> - I don't really like the behavior for any of the cases under 2.2.2. > > I especially don't like the fact that the output does not end in a > terminating newline for case 2.2.2.1.
This is exactly why I did tformat so that we do not have to have a complex special case (Jeff and I exchanged a few weatherbaloon patches on the list trying out heuristics) to avoid breaking existing scripts that use format.