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

[Mads Kiilerich]

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?

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