Patchwork wireproto: rephase the error message for unknown getbundle parameters

login
register
mail settings
Submitter Pierre-Yves David
Date June 2, 2014, 7:48 p.m.
Message ID <044630b9aa690782ee08.1401738525@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/4912/
State Accepted
Commit 7afe70a5d2ad5b22c21ba9be849451407c1f337f
Headers show

Comments

Pierre-Yves David - June 2, 2014, 7:48 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1401227131 25200
#      Tue May 27 14:45:31 2014 -0700
# Node ID 044630b9aa690782ee08f523b4b8735d99f05d57
# Parent  25732fab4dc35a76cfb8f659fde48c7e0dc42dbf
wireproto: rephase the error message for unknown getbundle parameters

In case of an unknown parameter passed to ``getbundle``, the server prints a
message and ignores the parameter. This message was misleadingly prefixed with
"abort: ". The use of "abort: " here is clearly wrong as nothing is actually
aborted and the ``getbundle`` call proceeds without the parameter.

The message is now prefixed with "warning: " and rephrased to make it clearer
that the parameter was simply ignored.
Augie Fackler - June 7, 2014, 1:40 p.m.
On Mon, Jun 02, 2014 at 12:48:45PM -0700, pierre-yves.david@ens-lyon.org wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1401227131 25200
> #      Tue May 27 14:45:31 2014 -0700
> # Node ID 044630b9aa690782ee08f523b4b8735d99f05d57
> # Parent  25732fab4dc35a76cfb8f659fde48c7e0dc42dbf
> wireproto: rephase the error message for unknown getbundle parameters
>
> In case of an unknown parameter passed to ``getbundle``, the server prints a
> message and ignores the parameter. This message was misleadingly prefixed with
> "abort: ". The use of "abort: " here is clearly wrong as nothing is actually
> aborted and the ``getbundle`` call proceeds without the parameter.
>
> The message is now prefixed with "warning: " and rephrased to make it clearer
> that the parameter was simply ignored.
>
> diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
> --- a/mercurial/wireproto.py
> +++ b/mercurial/wireproto.py
> @@ -505,11 +505,11 @@ def options(cmd, keys, others):
>      for k in keys:
>          if k in others:
>              opts[k] = others[k]
>              del others[k]
>      if others:
> -        sys.stderr.write("abort: %s got unexpected arguments %s\n"
> +        sys.stderr.write("warning: %s ignores unexpected arguments %s\n"
>                           % (cmd, ",".join(others)))

Now for the point where I'm going to argue about tenses briefly. Right
now this is stating that "method foo ignores arguments", but I think
you mean that arguments were already ignored, so should this be "%s
ignored" instead of "%s ignores"?

(The way you have it /might/ be intentional, thus my hesitance to just
queue this with a fix.)

>      return opts
>
>  # list of commands
>  commands = {}
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - June 7, 2014, 1:56 p.m.
On 06/07/2014 06:40 AM, Augie Fackler wrote:
> On Mon, Jun 02, 2014 at 12:48:45PM -0700, pierre-yves.david@ens-lyon.org wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>> # Date 1401227131 25200
>> #      Tue May 27 14:45:31 2014 -0700
>> # Node ID 044630b9aa690782ee08f523b4b8735d99f05d57
>> # Parent  25732fab4dc35a76cfb8f659fde48c7e0dc42dbf
>> wireproto: rephase the error message for unknown getbundle parameters
>>
>> In case of an unknown parameter passed to ``getbundle``, the server prints a
>> message and ignores the parameter. This message was misleadingly prefixed with
>> "abort: ". The use of "abort: " here is clearly wrong as nothing is actually
>> aborted and the ``getbundle`` call proceeds without the parameter.
>>
>> The message is now prefixed with "warning: " and rephrased to make it clearer
>> that the parameter was simply ignored.
>>
>> diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
>> --- a/mercurial/wireproto.py
>> +++ b/mercurial/wireproto.py
>> @@ -505,11 +505,11 @@ def options(cmd, keys, others):
>>       for k in keys:
>>           if k in others:
>>               opts[k] = others[k]
>>               del others[k]
>>       if others:
>> -        sys.stderr.write("abort: %s got unexpected arguments %s\n"
>> +        sys.stderr.write("warning: %s ignores unexpected arguments %s\n"
>>                            % (cmd, ",".join(others)))
>
> Now for the point where I'm going to argue about tenses briefly. Right
> now this is stating that "method foo ignores arguments", but I think
> you mean that arguments were already ignored, so should this be "%s
> ignored" instead of "%s ignores"?
>
> (The way you have it /might/ be intentional, thus my hesitance to just
> queue this with a fix.)

"ignored" seems better.
Augie Fackler - June 7, 2014, 5:23 p.m.
On Jun 7, 2014, at 9:56 AM, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:

> 
> 
> On 06/07/2014 06:40 AM, Augie Fackler wrote:
>> On Mon, Jun 02, 2014 at 12:48:45PM -0700, pierre-yves.david@ens-lyon.org wrote:
>>> # HG changeset patch
>>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>>> # Date 1401227131 25200
>>> #      Tue May 27 14:45:31 2014 -0700
>>> # Node ID 044630b9aa690782ee08f523b4b8735d99f05d57
>>> # Parent  25732fab4dc35a76cfb8f659fde48c7e0dc42dbf
>>> wireproto: rephase the error message for unknown getbundle parameters
>>> 
>>> In case of an unknown parameter passed to ``getbundle``, the server prints a
>>> message and ignores the parameter. This message was misleadingly prefixed with
>>> "abort: ". The use of "abort: " here is clearly wrong as nothing is actually
>>> aborted and the ``getbundle`` call proceeds without the parameter.
>>> 
>>> The message is now prefixed with "warning: " and rephrased to make it clearer
>>> that the parameter was simply ignored.
>>> 
>>> diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
>>> --- a/mercurial/wireproto.py
>>> +++ b/mercurial/wireproto.py
>>> @@ -505,11 +505,11 @@ def options(cmd, keys, others):
>>>      for k in keys:
>>>          if k in others:
>>>              opts[k] = others[k]
>>>              del others[k]
>>>      if others:
>>> -        sys.stderr.write("abort: %s got unexpected arguments %s\n"
>>> +        sys.stderr.write("warning: %s ignores unexpected arguments %s\n"
>>>                           % (cmd, ",".join(others)))
>> 
>> Now for the point where I'm going to argue about tenses briefly. Right
>> now this is stating that "method foo ignores arguments", but I think
>> you mean that arguments were already ignored, so should this be "%s
>> ignored" instead of "%s ignores"?
>> 
>> (The way you have it /might/ be intentional, thus my hesitance to just
>> queue this with a fix.)
> 
> "ignored" seems better.

Queueing with that fixed.

> 
> -- 
> Pierre-Yves David

Patch

diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
--- a/mercurial/wireproto.py
+++ b/mercurial/wireproto.py
@@ -505,11 +505,11 @@  def options(cmd, keys, others):
     for k in keys:
         if k in others:
             opts[k] = others[k]
             del others[k]
     if others:
-        sys.stderr.write("abort: %s got unexpected arguments %s\n"
+        sys.stderr.write("warning: %s ignores unexpected arguments %s\n"
                          % (cmd, ",".join(others)))
     return opts
 
 # list of commands
 commands = {}