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: >> 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.” >> - 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. 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. >> 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. >> 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-- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html