[Svnmerge] [PATCH] Indented log patch modified v6

[Raman Gupta]

Giovanni Bajo wrote:
> Raman Gupta <rocketraman at fastmail.fm> wrote:
> 
>> If we knew prefix_lines was never going to be called for anything else
>> in the future, I would agree. But I think for the marginal extra
>> complexity, we should keep the function handling both the newline
>> terminated case and the non-terminated case. At some point, someone
>> may want to call it with non-newline terminated text.
> 
> I don't like speculative programming, based on "what if". We're not writing a
> generic string manipulation library here. We're writing a function for a very
> specific case of usage, and it doesn't have to be overly generic. I don't see
> any problem in coercing our function to strings terminated by "\n", as long as
> this is documented (and better, asserted).

I like the version I gave, which is a one-liner AND handles both cases,
but not enough to hold up the commit on this.

> Anyway, notice that even if "svn log" didn't put a trailing "\n" in its output
> for some reason, we still *need* a trailing newline, otherwise "return
> longest_sep.join(logs)" will return a string in which one instance of the
> separator will not start at the beginning of a line. So the actual fix is to
> *add* a trailing "\n" if there isn't one and then call prefix_lines (which will
> contain the assert, and always succeed).

I know you wanted it in a separate commit, but I added Alan's suggestion
for the rstrip(message, \n) + \n because it does what you stated above
and ensures that there will be a terminating newline on the message
before prefix_lines is called. I added the note about removing trailing
newlines from the embedded log messages to the commit log message (see
below). I hope you are OK with this.

>> But I agree about simplifying it -- how about this instead:
>>
>>>>> def prefix_lines(prefix, lines):
>> ...     return re.compile(r"^(.*)$(\n?)", re.M) \
>>                     .sub(prefix + r"\1\2", lines)
>> ...
>>>>> prefix_lines("zz", "foo")
>> 'zzfoo'
>>>>> prefix_lines("zz", "foo\n")
>> 'zzfoo\n'
>>>>> prefix_lines("zz", "foo\nbar")
>> 'zzfoo\nzzbar'
>>>>> prefix_lines("zz", "foo\nbar\n")
>> 'zzfoo\nzzbar\n'
> 
> It's much better than before, and it would be ok. But if my analysis above is
> correct, we can use my version which I still find easier to read.

Attached, version 6 of the patch.

Commit log msg:

Indent the merged revisions' log messages when creating the commit log
message after executing svn merge. The embedded log messages have their
trailing newlines stripped.

Patch by: Alan Barrett <apb at cequrux.com>
          Raman Gupta <rocketraman at fastmail.fm>
Reviewed by: Giovanni Bajo <rasky at develer.com>

* contrib/client-side/svnmerge.py
  (prefix_lines):
    New method to take a string and prepend each line of the string
      with a specified string. Lines are delimited by newline
      characters, and the entire string is asserted to be newline
      terminated.
  (construct_merged_log_message):
    Added indentation for each commit message. Added check for empty
      log messages, which should never happen but doesn't hurt.

* contrib/client-side/svnmerge_test.py
  (TestCase_PrefixLines.test_basic):
    New test for the prefix_lines functionality.

Cheers,
Raman
-------------- next part --------------
A non-text attachment was scrubbed...
Name: log-indent-v6.patch
Type: text/x-patch
Size: 2649 bytes
Desc: not available
Url : /pipermail/svnmerge/attachments/20060302/0006fda7/attachment.bin