[Svnmerge] [PATCH] Indented log patch modified v5

[Raman Gupta]

Alan Barrett wrote:
> On Wed, 01 Mar 2006, Raman Gupta wrote:
>>>> +def prefix_lines(prefix, lines):
>>>> +  """Given a string representing lines of text,
>>>> +  insert the specified prefix at the begining of each line,
>>>> +  and return the result."""
>>>> +  
>>>> +  if len(lines) > 0:
>>>> +      return "\n".join([prefix + L for L in lines.split("\n")])
>>>> +  else:
>>>> +      return ""
>>> This function assumes that the last line will not be terminated by "\n".
>>> That assumption should be documented.
>> The function doesn't really assume anything -- each line that is passed
>> in will be indented. That includes any zero length lines at the end.
> 
> We seem to have a disagreement about the definition of a "line".
> In the string "one\ntwo\n", I say that there are two lines, each
> terminated by "\n".  (You seem to think that there's a third
> zero-length line.)  If you want to indent those two lines (per my
> definition of lines) to get "  one\n  two\n", then you need to
> remove a "\n" before calling prefix_lines, and add it back later,
> like this: prefix_lines("  ","one\ntwo")+"\n".

Ok.

>>>> -        for match in LOG_SEPARATOR_RE.findall(message):
>>>> -            sep = match[1]
>>>> -            if len(sep) > len(longest_sep):
>>>> -                longest_sep = sep
>>>> +        if len(message) > 0:
>>>> +            logs.append("\n" + prefix_lines(LOG_LINE_PREFIX, \
>>>                            ^^^^
>>>> +                                            rstrip(message, "\n")) + "\n")
>>>> +            for match in LOG_SEPARATOR_RE.findall(message):
>>>> +                sep = match[1]
>>>> +                if len(sep) > len(longest_sep):
>>>> +                    longest_sep = sep
>>> I haven't run the code, but I think that the "\n" highlighted above will
>>> put an unwanted blank line between the "........" and the first line of
>>> the nested log message.
>> No, it doesn't. Try it out.
> 
> I tried it, and it does insert a blank line as I expected.

You're right, I must need new glasses :-)  Fixed.

> I append my attempt.  I made prefix_lines work with or without a "\n"
> at the end of the last line (but if you really want a blank line at the
> end, you can't omit the "\n" after it).

> +def prefix_lines(prefix, lines):
> +  """Given a string representing one or more lines of text, insert the
> +  specified prefix at the beginning of each line, and return the result.
> +  Each line of the result will be terminated by "\n", even if the "\n"
> +  was missing from the last line of input."""
> +  
> +  lineslist = lines.split("\n")
> +  if len(lineslist) > 1 and lineslist[-1] == "":
> +      # remove unwanted extra element representing the nothingness
> +      # after the last "\n" in the input string
> +      pop(lineslist)
> +  return "\n".join([prefix + L for L in lineslist]) + "\n"
> +

One thing I don't like. Your prefix lines is inconsistent with the
newlines. If the value passed has a newline at the end, it returns it.
If the value passed does not have a newline at the end, it appends one.

In the new patch I attached, I have used your new function, but changed
the return value to be consistent with the passed value, as well as
updated the test case. Note that I also fixed your indentation, and
changed pop(lineslist) to lineslist.pop(). Is pop a recently added
function that we should have a local replacement for?

Cheers,
Raman
-------------- next part --------------
A non-text attachment was scrubbed...
Name: log-indent-v5.patch
Type: text/x-patch
Size: 3407 bytes
Desc: not available
Url : /pipermail/svnmerge/attachments/20060301/cfed9d7d/attachment.bin