Re: [PATCH] Support wrapping commit messages when you read them

[Junio C Hamano <gitster@pobox.com>]

Sidney San Martín <s@sidneysm.com> writes:

> Fairly simpleminded line wrapping that makes commit messages
> readable if they weren’t wrapped by the committer.

This does not say anything useful, other than "this is a naïve
implementation of message wrapper" and invites "So what?".

The most simple-minded solution is to reject such commits with crappy log
message.

After all, SCM is merely a method to help communication between
developers, and sticking to the common denominator is a proven good way to
make sure everybody involved in the project can use what is recorded in
the repository. This is not limited only to the log message, but equally
applies to filenames (e.g. don't create xt_tcpmss.c and xt_TCPMSS.c in the
same directory if you want your project extractable on case insensitive
filesystems) and even to the sources.

You need to justify the cause a bit better. Why is such a new logic
justified?

> - Use strbuf_add_wrapped_text() to do the dirty work
> - Detect simple lists which begin with "+ ", "* ", or "- " and indent
>   them appropriately (like this line)
> - Print lines which begin with whitespace as-is (e.g. code samples)

I suspect the above would make it more palatable than format=flowed
brought up in earlier discussions, which is unsuitable for nothing but
straight text.

> Add --wrap[=<width>] and --no-wrap to commands that pretty-print commit
> messages, and add log.wrap and log.wrap.width configuration options.

Why do you need two separate options and configurations that look as if
they are independent but in reality not?  If you say "no wrap", there is
no room for you to say "wrap width is 72".

I would expect something like:

 --log-message-wrap, --log-message-wrap=72, --log-message-wrap=no 

with --log-message-wrap=yes as a synonym for --log-message-wrap to give
consistency. The corresponding configuraiton would be log.messageWrap
whose values could be the usual bool-or-int.

> log.wrap defaults to never, and can be set to never/false, auto/true,
> or always. If auto, hijack want_color() to decide whether it’s
> appropriate to use line wrapping. (This is a little hacky, but as far
> as I can tell the conditions for auto color and auto wrapping are the
> same.

Why does coloring have _anything_ to do with line wrapping? Maybe your
personaly preference might be "wrap and color if interactive terminal" but
that is conflating two unrelated concepts. A user may not expect coloring
on a dumb interactive terminal, but wrapping may still be useful.

> log.wrap.width defaults to 80.

This does not deserve a comment as I already rejected the "two
configuration" approach, but do not use three-level names this way. We try
to reserve three-level names only for cases where the second level is used
for an unbound collection (e.g. "remote.$name.url", "branch.$name.merge").
that is user-specified.