Denton Liu <liu.denton@xxxxxxxxx> writes:
> Subject: Re: [PATCH 6/8] reflog-walk.c: don't print last newline with oneline
Please find a better phrasing that does not mislead the readers to
think "eek, so now output from 'git log -g --oneline' does not get
LF terminated?"
I think you are just changing the place where the LF is given from
the callee that shows a single record (and then used to show LF
after the record) to the caller that calls the callee (and now it
shows LF after the call returns), in order to preserve the output,
but with the given proposed log message, it is not clear if that is
what you meant to do (I can read from the code what _is_ going on,
but the log is to record what you _wanted_ to do, which can be
different).
> @@ -661,8 +661,10 @@ void show_log(struct rev_info *opt)
In the pre-context before this line there is a guard to ensure that
we do this only when showing an entry from the reflog, so the effect
of this hunk is "when showing from reflog in --oneline format, show
LF after showing a single record" ...
> opt->commit_format == CMIT_FMT_ONELINE,
> &opt->date_mode,
> opt->date_mode_explicit);
> - if (opt->commit_format == CMIT_FMT_ONELINE)
> + if (opt->commit_format == CMIT_FMT_ONELINE) {
> + putc('\n', opt->diffopt.file);
> return;
> + }
> }
> }
>
> diff --git a/reflog-walk.c b/reflog-walk.c
> index 3a25b27d8f..e2b4c0b290 100644
> --- a/reflog-walk.c
> +++ b/reflog-walk.c
> @@ -285,7 +285,11 @@ void show_reflog_message(struct reflog_walk_info *reflog_info, int oneline,
> info = &commit_reflog->reflogs->items[commit_reflog->recno+1];
> get_reflog_selector(&selector, reflog_info, dmode, force_date, 0);
> if (oneline) {
> - printf("%s: %s", selector.buf, info->message);
> + struct strbuf message = STRBUF_INIT;
> + strbuf_addstr(&message, info->message);
> + strbuf_trim_trailing_newline(&message);
> + printf("%s: %s", selector.buf, message.buf);
> + strbuf_release(&message);
... and this one is what gets called from the above caller; we lost
the final LF (if there is) from the output, that is compensated by
the caller.
How well do we work with the "-z" output format, by the way, with
the hardcoded '\n' on the output codepath?
Making a new allocation feels inefficient just to omit the trailing
newlines. I wonder if a helper function that takes a "const char *"
and returns a "const char *" in that string that points at the CRLF
or LF or NUL at its end would be warranted. If we do not care about
CRLF, this part would become something like...
if (oneline) {
const char *endp = strchrnul(info->message, '\n');
printf("%s: %.*s", selector.buf,
(int)(endp - info->message), info->message);
}
but I didn't trace the code/data flow to see where info->message
comes from well enough to tell if we need to worry about CRLF. I
thought reflogs are always written with LF termination, though.
Thanks.
