[Svnmerge] [PATCH] Indented log patch modified v5

[Giovanni Bajo]

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).

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).


> 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.

Giovanni Bajo