Sidney San Martín [off-list ref] 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.
Hey Junio,
I apologize for the delay, I do want to keep working on this idea. I had to put it down but should have more time now if you're willing to keep talking about it.
Thanks for taking the time to look at my first patch.
On Dec 25, 2011, at 4:57 AM, Junio C Hamano wrote:
quoted
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?
You’re right, that sentence doesn't say anything.
I agree that projects need to have standards for their commit messages, but I also think that line wrapping should be taken care of by the computer so that the humans can think about the content of their commit messages. It's easier for everyone.
It also makes sense to not assume the user is using an 80-column terminal. Like I mentioned in another email, other tools work this way (e.g. manpages). It turns out that "git help" already has code to detect the width of the terminal, and it formats its output to fit it. I want to adapt that logic for this feature.
How about replacing that paragraph with this:
“Git didn’t previously support formatting commit messages for a user’s terminal, and the common practice has been to pre-wrap commit messages to under 80 columns. This is necessary for some projects, especially those which trade patches over email where mail clients might damage longer lines, but in many cases it’s only done so that the messages are readable in "git log" and the like. Supporting line wrapping in git lets users choose to leave their commit messages unwrapped and have them formatted for their terminal when displayed.”
quoted
- 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.
quoted
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.
I stole this from other options: --progress/--no-progress, --color/--color=[<when>]/--no-color, --track/--no-track, etc.
The separate wrap/wrap.width config options were so that you could set it separately to auto or always and also specify a width. But, I don't know if that's needed anymore. See below.
quoted
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.
It doesn’t — I used want_color() so that I could get the patch out there without making other changes to the codebase or duplicating its code, so you could comment on the rest of it. I'll get it out of the next version of the patch, which I'll try to get to you later today or tomorrow.
quoted
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.
OK, that was a misunderstanding on my part. Actually, I would be in favor of getting rid of that option completely, moving in support for detecting the terminal width from "git help" and making it just a boolean, auto-only. How does that sound?
Sidney