Patchwork [1,of,1] largefiles: improve 'unexpected response' warning newlines

login
register
mail settings
Submitter Simon Heimberg
Date Nov. 19, 2013, 12:53 a.m.
Message ID <7b1b6e0c00f66bc523a4.1384822420@lapsi.heimberg.home>
Download mbox | patch
Permalink /patch/3064/
State Superseded
Headers show

Comments

Simon Heimberg - Nov. 19, 2013, 12:53 a.m.
# HG changeset patch
# User Simon Heimberg <simohe@besonet.ch>
# Date 1384821608 -3600
#      Tue Nov 19 01:40:08 2013 +0100
# Node ID 7b1b6e0c00f66bc523a4b0113946d106b6442d67
# Parent  08fffc33af47f3f47647fd5df6d8d76f71d7f38d
largefiles: improve 'unexpected response' warning newlines

Warnings should always end with \n. The warning message might contain or end
with \n. But insetead of escaping all, print one and strip any existing.

On windows, paths looks much better like this than with escaped backslashes
(intruduced in 2a03faf8b5fe). And many test failures on windows will be fixed.
Mads Kiilerich - Nov. 19, 2013, 4:53 p.m.
On 11/18/2013 07:53 PM, Simon Heimberg wrote:
> # HG changeset patch
> # User Simon Heimberg <simohe@besonet.ch>
> # Date 1384821608 -3600
> #      Tue Nov 19 01:40:08 2013 +0100
> # Node ID 7b1b6e0c00f66bc523a4b0113946d106b6442d67
> # Parent  08fffc33af47f3f47647fd5df6d8d76f71d7f38d
> largefiles: improve 'unexpected response' warning newlines
>
> Warnings should always end with \n. The warning message might contain or end
> with \n. But insetead of escaping all, print one and strip any existing.
>
> On windows, paths looks much better like this than with escaped backslashes
> (intruduced in 2a03faf8b5fe). And many test failures on windows will be fixed.

Which test failures? There is no test coverage of this line of code, AFAICS.

This message is for debugging the situation where the server really 
sends some unexpected crap back. It will never be a nice message anyway 
but we will like to get every non-ascii character shown in an 
unambiguous way so an expert can see what really happened and debug it.

/Mads

> diff -r 08fffc33af47 -r 7b1b6e0c00f6 hgext/largefiles/proto.py
> --- a/hgext/largefiles/proto.py	Sun Nov 17 20:22:59 2013 -0500
> +++ b/hgext/largefiles/proto.py	Tue Nov 19 01:40:08 2013 +0100
> @@ -96,7 +96,7 @@
>                           self.ui.warn(_('remote: '), l) # assume l ends with \n
>                       return int(d)
>                   except (ValueError, urllib2.HTTPError):
> -                    self.ui.warn(_('unexpected putlfile response: %r\n') % res)
> +                    self.ui.warn(_('unexpected putlfile response: %s\n') % res.rstrip('\n'))
>                       return 1
>               # ... but we can't use sshrepository._call because the data=
>               # argument won't get sent, and _callpush does exactly what we want
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Simon Heimberg - Nov. 19, 2013, 7:17 p.m.
Am 19.11.2013 17:53, schrieb Mads Kiilerich:
> On 11/18/2013 07:53 PM, Simon Heimberg wrote:
>> # HG changeset patch
>> # User Simon Heimberg <simohe@besonet.ch>
>> # Date 1384821608 -3600
>> #      Tue Nov 19 01:40:08 2013 +0100
>> # Node ID 7b1b6e0c00f66bc523a4b0113946d106b6442d67
>> # Parent  08fffc33af47f3f47647fd5df6d8d76f71d7f38d
>> largefiles: improve 'unexpected response' warning newlines
>>
>> Warnings should always end with \n. The warning message might contain 
>> or end
>> with \n. But insetead of escaping all, print one and strip any existing.
>>
>> On windows, paths looks much better like this than with escaped 
>> backslashes
>> (intruduced in 2a03faf8b5fe). And many test failures on windows will 
>> be fixed.
>
> Which test failures? There is no test coverage of this line of code, 
> AFAICS.

You are right. The beginning of the failing message is not "unexpected 
putlfile response:", it is something like "large3: largefile %s not 
available from %s", which is totally different. The only similarity is 
that something is encoded, but it is repr here and url-encoding there.

Sorry, I reject my patch. I will dig on, hopefully more successful.

>
> This message is for debugging the situation where the server really 
> sends some unexpected crap back. It will never be a nice message 
> anyway but we will like to get every non-ascii character shown in an 
> unambiguous way so an expert can see what really happened and debug it.
>
> /Mads
>
>> diff -r 08fffc33af47 -r 7b1b6e0c00f6 hgext/largefiles/proto.py
>> --- a/hgext/largefiles/proto.py    Sun Nov 17 20:22:59 2013 -0500
>> +++ b/hgext/largefiles/proto.py    Tue Nov 19 01:40:08 2013 +0100
>> @@ -96,7 +96,7 @@
>>                           self.ui.warn(_('remote: '), l) # assume l 
>> ends with \n
>>                       return int(d)
>>                   except (ValueError, urllib2.HTTPError):
>> -                    self.ui.warn(_('unexpected putlfile response: 
>> %r\n') % res)
>> +                    self.ui.warn(_('unexpected putlfile response: 
>> %s\n') % res.rstrip('\n'))
>>                       return 1
>>               # ... but we can't use sshrepository._call because the 
>> data=
>>               # argument won't get sent, and _callpush does exactly 
>> what we want
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff -r 08fffc33af47 -r 7b1b6e0c00f6 hgext/largefiles/proto.py
--- a/hgext/largefiles/proto.py	Sun Nov 17 20:22:59 2013 -0500
+++ b/hgext/largefiles/proto.py	Tue Nov 19 01:40:08 2013 +0100
@@ -96,7 +96,7 @@ 
                         self.ui.warn(_('remote: '), l) # assume l ends with \n
                     return int(d)
                 except (ValueError, urllib2.HTTPError):
-                    self.ui.warn(_('unexpected putlfile response: %r\n') % res)
+                    self.ui.warn(_('unexpected putlfile response: %s\n') % res.rstrip('\n'))
                     return 1
             # ... but we can't use sshrepository._call because the data=
             # argument won't get sent, and _callpush does exactly what we want