[PATCH] run-tests: introduce ' (cr)\n' markup of bare \r in test output

[Augie Fackler]

On Sep 26, 2012, at 4:34 AM, Mads Kiilerich <mads at kiilerich.com> wrote:

> On 09/26/2012 02:32 AM, Augie Fackler wrote:
>> On Sep 25, 2012, at 7:09 PM, Mads Kiilerich wrote:
>> 
>>> # HG changeset patch
>>> # User Mads Kiilerich <mads at kiilerich.com>
>>> # Date 1348613787 -7200
>>> # Node ID bdc65acbd3d003f184bf0594ab406e20b2f36ca4
>>> # Parent  d34ba4991188fdfa4f8322b9a32c4c08aaf6ff27
>>> run-tests: introduce ' (cr)\n' markup of bare \r in test output
> 
> A couple of additional thoughts:
> 
> We could probably use something like '\r (noeol) (esc)' instead of introducing '(cr)'. It would be conceptually simpler but add more clutter to the tests.
> 
> Another solution could be to change '(esc)' so the trailing '\n' that currently is implicit always was explicit ... and could be left out if the lines were split for other reasons, i.e. \r.
> 
> Opinions?

I think I like the (noeol) option the best, rather than making (esc) imply a missing EOL.

> 
>> 
>>> diff --git a/tests/run-tests.py b/tests/run-tests.py
>>> --- a/tests/run-tests.py
>>> +++ b/tests/run-tests.py
>>> @@ -498,6 +498,20 @@
>>>     vlog("# Running", cmd)
>>>     return run(cmd, wd, options, replacements)
>>> 
>>> +needcrsplit = re.compile(r'([\x20-\x7e]*\r)+[\x20-\x7e]*\n$').match
>>> +crsplit = re.compile(r'\r(?!\n)').split
>>> +def crescape(ls):
>>> +    """yield the lf terminated strings from ls, possibly split on bare cr and
>>> +    with ' (cr)' markup"""
>> This function feels misnamed to me - it's both splitting the input on \n and escaping \r? Could we do these as separate passes? I know it'd be slightly slower, but it feels cleaner to me to just thing.sub('\r', ' (cr)\n') as needed.
> 
> No, the input is already split on \n - either because the output line ends with \n or by the \n following the salt. This function will only split on \r when not followed by \n. \r\n will thus not be changed and will still be shown as '\r (esc)'.
> 
> But yes, cr is really not escaped by this function. 'crmarkup' would perhaps be more descriptive ... and certainly better than 'crsplit1' and 'crsplit2'.
> 
> The splitting on \r and markup of the \r could be done in separate passes or elsewhere. That would probably be slightly cleaner, but it seems like the best way to do it depends on exactly which markup we want.
> 
> /Mads
> 
>>> +    for l in ls:
>>> +        if needcrsplit(l):
>>> +            parts = crsplit(l)
>>> +            for s in parts[:-1]:
>>> +                yield s + ' (cr)\n'
>>> +            yield parts[-1]
>>> +        else:
>>> +            yield l
>>> +
>>> needescape = re.compile(r'[\x00-\x08\x0b-\x1f\x7f-\xff]').search
>>> escapesub = re.compile(r'[\x00-\x08\x0b-\x1f\\\x7f-\xff]').sub
>>> escapemap = dict((chr(i), r'\x%02x' % i) for i in range(256))
>>> @@ -684,7 +698,7 @@
>>>     pos = -1
>>>     postout = []
>>>     ret = 0
>>> -    for n, l in enumerate(output):
>>> +    for l in crescape(output):
>>>         lout, lcmd = l, None
>>>         if salt in l:
>>>             lout, lcmd = l.split(salt, 1)
>>> diff --git a/tests/test-archive.t b/tests/test-archive.t
>> 
>> [elided rest of patch]
>> 
>