Re: [PATCH][repost] Don't add extra blank line to log messages

Greg Hudson wrote:
>
> I think it's uncharitable to call it "pilot error"; it's a matter of
> taste how we should be handling newlines in log messages.

Yes. We already do certain transformations, such as converting all of the line endings to the native type, and, if the message is provided from an editor, converting an all-white-space message to an empty message.

I agree that adding blank lines to log messages is generally an ugly thing to do. (If one wants an extra blank line before the row of dashes in "svn log" output, that's a separate thing that should not be counted in the log message line count nor be output by "svn propget svn:log" etc.)

> I'd say the best behavior would be to ensure at commit time that there
> is a newline at the end of the log message, and to only put an extra
> newline at the end of the message in "svn log" if there isn't one
> already there.

That sounds like a nice solution. "svn propget svn:log --strict" could still output the exact store message.

When I brought up something like this before, I was told that we wanted the exact form of the originally provided log message to be recoverable from "svn log" output, including whether it ended with a newline, but I am not sure that that is really a very useful thing to do.

I would very much like the "svn log" output to count a one-line log message as "1 line" and not add a blank line and count it as "2 lines".

I'd also say, make sure that there is _only_ one newline at the end of the message, and strip any extra white space. This particularly makes sense when the message has been written above the marker line.

The Subversion project has a policy of always writing a log message, but some projects don't and so I would like to see empty log messages handled more neatly in the "svn log" output: instead of

------------------------------------------------------------------------
r91 | julianfoad | 2004-03-26 10:47:05 +0000 (Fri, 26 Mar 2004) | 1 line

------------------------------------------------------------------------

I would prefer to see

------------------------------------------------------------------------
r91 | julianfoad | 2004-03-26 10:47:05 +0000 (Fri, 26 Mar 2004) | 0 lines

------------------------------------------------------------------------

or, even better,

------------------------------------------------------------------------
r91 | julianfoad | 2004-03-26 10:47:05 +0000 (Fri, 26 Mar 2004) | 0 lines
------------------------------------------------------------------------

Note that this latter form is still programmatically parseable, but requires an extra step: "if 0 lines then don't expect a blank separator line". The attached patch changes the display of blank log messages to this latter form, without affecting the display of other log messages. I haven't posted it before because I think we should form a consensus on all of these log message blank line issues before making a change.

I don't know whether we will alow ourselves to change this now until version 2, but those of us who are interested might as well discuss what we would like to see.

- Julian

If the log message is blank, then don't display it or the extra blank line.

Index: subversion/clients/cmdline/log-cmd.c
===================================================================
--- subversion/clients/cmdline/log-cmd.c (revision 8659)
+++ subversion/clients/cmdline/log-cmd.c (working copy)
@@ -45,9 +45,10 @@
 /* Helper for log_message_receiver().
  *
  * Return the number of lines in MSG, allowing any kind of newline
- * termination (CR, CRLF, or LFCR), even inconsistent. The minimum
- * number of lines in MSG is 1 -- even the empty string is considered
- * to have one line, due to the way we print log messages.
+ * termination (CR, CRLF, or LFCR), even inconsistent. This counts
+ * the number of lines that are going to be used to display this message,
+ * which will include an extra blank line if the message already ends with
+ * a newline.
  */
 static int
 num_lines (const char *msg)
@@ -55,6 +56,9 @@
   int count = 1;
   const char *p;
 
+ if (*msg == '\0')
+ return 0;
+
   for (p = msg; *p; p++)
     {
       if (*p == '\n')
@@ -230,14 +234,14 @@
   if (! lb->omit_log_message)
     {
       if (msg == NULL)
- msg = "";
-
- {
- /* Convert log message from UTF8/LF to native locale and eol-style. */
- svn_string_t *logmsg = svn_string_create (msg, pool);
- SVN_ERR (svn_subst_detranslate_string (&logmsg, logmsg, TRUE, pool));
- msg_stdout = logmsg->data;
- }
+ msg_stdout = "";
+ else
+ {
+ /* Convert log message from UTF8/LF to native locale and eol-style. */
+ svn_string_t *logmsg = svn_string_create (msg, pool);
+ SVN_ERR (svn_subst_detranslate_string (&logmsg, logmsg, TRUE, pool));
+ msg_stdout = logmsg->data;
+ }
     }
 
   SVN_ERR (svn_stream_printf (lb->out, pool, SEP_STRING));
@@ -249,9 +253,10 @@
   if (! lb->omit_log_message)
     {
       lines = num_lines (msg_stdout);
+ /* printf("### log msg: [%s]: %d lines\n", msg_stdout, lines); */
       SVN_ERR (svn_stream_printf (lb->out, pool,
                                   " | %d line%s", lines,
- (lines > 1) ? "s" : ""));
+ (lines != 1) ? "s" : ""));
     }
 
   SVN_ERR (svn_stream_printf (lb->out, pool, APR_EOL_STR));
@@ -295,7 +300,7 @@
         }
     }
 
- if (! lb->omit_log_message)
+ if (lines && ! lb->omit_log_message)
     {
       /* A blank line always precedes the log message. */
       SVN_ERR (svn_stream_printf (lb->out, pool, APR_EOL_STR "%s" APR_EOL_STR,

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Mar 28 12:58:50 2004