Patchwork extensions: allow replacing command synopsis

login
register
mail settings
Submitter Ryan McElroy
Date Feb. 11, 2015, 3:43 a.m.
Message ID <dac1fbb56786c2033e18.1423626192@devbig105.prn2.facebook.com>
Download mbox | patch
Permalink /patch/7783/
State Superseded
Delegated to: Augie Fackler
Headers show

Comments

Ryan McElroy - Feb. 11, 2015, 3:43 a.m.
# HG changeset patch
# User Ryan McElroy <rm@fb.com>
# Date 1423508565 28800
#      Mon Feb 09 11:02:45 2015 -0800
# Node ID dac1fbb56786c2033e188e7b762a9429cd7ff621
# Parent  ff5caa8dfd993680d9602ca6ebb14da9de10d5f4
extensions: allow replacing command synopsis
Augie Fackler - Feb. 11, 2015, 2:37 p.m.
On Feb 10, 2015, at 10:43 PM, Ryan McElroy <rm@fb.com> wrote:

> # HG changeset patch
> # User Ryan McElroy <rm@fb.com>
> # Date 1423508565 28800
> #      Mon Feb 09 11:02:45 2015 -0800
> # Node ID dac1fbb56786c2033e188e7b762a9429cd7ff621
> # Parent  ff5caa8dfd993680d9602ca6ebb14da9de10d5f4
> extensions: allow replacing command synopsis

Hm, interesting. Can you give a sample reason why we should support this?

> 
> diff --git a/mercurial/extensions.py b/mercurial/extensions.py
> --- a/mercurial/extensions.py
> +++ b/mercurial/extensions.py
> @@ -152,7 +152,7 @@ def afterloaded(extension, callback):
>     else:
>         _aftercallbacks.setdefault(extension, []).append(callback)
> 
> -def wrapcommand(table, command, wrapper):
> +def wrapcommand(table, command, wrapper, synopsis=None):
>     '''Wrap the command named `command' in table
> 
>     Replace command in the command table with wrapper. The wrapped command will
> @@ -182,6 +182,8 @@ def wrapcommand(table, command, wrapper)
> 
>     newentry = list(entry)
>     newentry[0] = wrap
> +    if synopsis is not None:
> +        newentry[2] = synopsis
>     table[key] = tuple(newentry)
>     return entry
> 
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Ryan McElroy - Feb. 11, 2015, 4:32 p.m.
On 2/11/2015 6:37 AM, Augie Fackler wrote:
> On Feb 10, 2015, at 10:43 PM, Ryan McElroy <rm@fb.com> wrote:
>
>> # HG changeset patch
>> # User Ryan McElroy <rm@fb.com>
>> # Date 1423508565 28800
>> #      Mon Feb 09 11:02:45 2015 -0800
>> # Node ID dac1fbb56786c2033e188e7b762a9429cd7ff621
>> # Parent  ff5caa8dfd993680d9602ca6ebb14da9de10d5f4
>> extensions: allow replacing command synopsis
> Hm, interesting. Can you give a sample reason why we should support this?
>
In general, this will allow extensions that wrap/extend commands to be 
better integrated.

Specifically, in the remotenames extension, we're adding some new flags 
to commands -- some examples are --to to the push command, and --all and 
--remote to bookmarks. Today, you can't edit the synopsis string in the 
command tuple because tuples and strings are both immutable. The list of 
args is extensible because it's a list (and editing a list doens't 
change the tuple that contains it). However, we are unable to add the 
new flags to the command synopsis. This would allow us to do that.

I can update the commit message to include these reasons and send out a 
V2 -- let me know.
Augie Fackler - Feb. 11, 2015, 5:26 p.m.
On Feb 11, 2015, at 11:32 AM, Ryan McElroy <rm@fb.com> wrote:

> 
> On 2/11/2015 6:37 AM, Augie Fackler wrote:
>> On Feb 10, 2015, at 10:43 PM, Ryan McElroy <rm@fb.com> wrote:
>> 
>>> # HG changeset patch
>>> # User Ryan McElroy <rm@fb.com>
>>> # Date 1423508565 28800
>>> #      Mon Feb 09 11:02:45 2015 -0800
>>> # Node ID dac1fbb56786c2033e188e7b762a9429cd7ff621
>>> # Parent  ff5caa8dfd993680d9602ca6ebb14da9de10d5f4
>>> extensions: allow replacing command synopsis
>> Hm, interesting. Can you give a sample reason why we should support this?
>> 
> In general, this will allow extensions that wrap/extend commands to be better integrated.
> 
> Specifically, in the remotenames extension, we're adding some new flags to commands -- some examples are --to to the push command, and --all and --remote to bookmarks. Today, you can't edit the synopsis string in the command tuple because tuples and strings are both immutable. The list of args is extensible because it's a list (and editing a list doens't change the tuple that contains it). However, we are unable to add the new flags to the command synopsis. This would allow us to do that.
> 
> I can update the commit message to include these reasons and send out a V2 -- let me know.

Please do. A sample of a synopsis change that remotebranches might be nice too, so that future readers will know the complete rationale.
Gregory Szorc - Feb. 11, 2015, 7:53 p.m.
On Wed, Feb 11, 2015 at 8:32 AM, Ryan McElroy <rm@fb.com> wrote:

>
> On 2/11/2015 6:37 AM, Augie Fackler wrote:
>
>> On Feb 10, 2015, at 10:43 PM, Ryan McElroy <rm@fb.com> wrote:
>>
>>  # HG changeset patch
>>> # User Ryan McElroy <rm@fb.com>
>>> # Date 1423508565 28800
>>> #      Mon Feb 09 11:02:45 2015 -0800
>>> # Node ID dac1fbb56786c2033e188e7b762a9429cd7ff621
>>> # Parent  ff5caa8dfd993680d9602ca6ebb14da9de10d5f4
>>> extensions: allow replacing command synopsis
>>>
>> Hm, interesting. Can you give a sample reason why we should support this?
>>
>>  In general, this will allow extensions that wrap/extend commands to be
> better integrated.
>
> Specifically, in the remotenames extension, we're adding some new flags to
> commands -- some examples are --to to the push command, and --all and
> --remote to bookmarks. Today, you can't edit the synopsis string in the
> command tuple because tuples and strings are both immutable. The list of
> args is extensible because it's a list (and editing a list doens't change
> the tuple that contains it). However, we are unable to add the new flags to
> the command synopsis. This would allow us to do that.
>
> I can update the commit message to include these reasons and send out a V2
> -- let me know.
>

Instead of direct docstring manipulation, how about an API to add
extension-specific "sections" to the docstring. This could result in
something like the following, without concerns that extensions would
interfere with each other.

hg push [-f] [-r REV]... [-e CMD] [--remotecmd CMD] [DEST]

push changes to the specified destination

    Push changesets from the local repository to the specified destination.

    ...

    Returns 0 if push was successful, 1 if nothing to push.

    pushrebase extension

        pushrebase will enable "hg push" to automatically rebase pushed
changes on the server.

    firefoxtree extension

        firefoxtree changes the behavior of pushes to Firefox repositories
so the default push revision is ".". This will prevent
        attempted pushes of multiple heads, which would only get rejected
due to a server-side hook.

options ([+] can be repeated):

 -f --force                 force push
 -r --rev REV [+]           a changeset intended to be included in the
Ryan McElroy - Feb. 11, 2015, 8:17 p.m.
On 2/11/2015 11:53 AM, Gregory Szorc wrote:
> On Wed, Feb 11, 2015 at 8:32 AM, Ryan McElroy <rm@fb.com 
> <mailto:rm@fb.com>> wrote:
>
>
>     On 2/11/2015 6:37 AM, Augie Fackler wrote:
>
>         On Feb 10, 2015, at 10:43 PM, Ryan McElroy <rm@fb.com
>         <mailto:rm@fb.com>> wrote:
>
>             # HG changeset patch
>             # User Ryan McElroy <rm@fb.com <mailto:rm@fb.com>>
>             # Date 1423508565 28800
>             #      Mon Feb 09 11:02:45 2015 -0800
>             # Node ID dac1fbb56786c2033e188e7b762a9429cd7ff621
>             # Parent  ff5caa8dfd993680d9602ca6ebb14da9de10d5f4
>             extensions: allow replacing command synopsis
>
>         Hm, interesting. Can you give a sample reason why we should
>         support this?
>
>     In general, this will allow extensions that wrap/extend commands
>     to be better integrated.
>
>     Specifically, in the remotenames extension, we're adding some new
>     flags to commands -- some examples are --to to the push command,
>     and --all and --remote to bookmarks. Today, you can't edit the
>     synopsis string in the command tuple because tuples and strings
>     are both immutable. The list of args is extensible because it's a
>     list (and editing a list doens't change the tuple that contains
>     it). However, we are unable to add the new flags to the command
>     synopsis. This would allow us to do that.
>
>     I can update the commit message to include these reasons and send
>     out a V2 -- let me know.
>
>
> Instead of direct docstring manipulation, how about an API to add 
> extension-specific "sections" to the docstring. This could result in 
> something like the following, without concerns that extensions would 
> interfere with each other.
>
> hg push [-f] [-r REV]... [-e CMD] [--remotecmd CMD] [DEST]
>
> push changes to the specified destination
>
>     Push changesets from the local repository to the specified 
> destination.
>
>     ...
>
>     Returns 0 if push was successful, 1 if nothing to push.
>
>     pushrebase extension
>
>         pushrebase will enable "hg push" to automatically rebase 
> pushed changes on the server.
>
>     firefoxtree extension
>
>         firefoxtree changes the behavior of pushes to Firefox 
> repositories so the default push revision is ".". This will prevent
>         attempted pushes of multiple heads, which would only get 
> rejected due to a server-side hook.
>
> options ([+] can be repeated):
>
>  -f --force                 force push
>  -r --rev REV [+]           a changeset intended to be included in the
>
Note that this patch doesn't touch docstrings, only synopses, which are 
the shorter version of help.

However, you are right -- the "correct" way to do this is to introduce 
ways for extensions to more intelligently manipulate both the synopsis 
and the docstring, most likely through appending (but an argument might 
be made to allow other manipulations as well).

For now, appending will suffice for my needs, so I'll send out a series 
that enables appending only to both the synopsis and the docstring.

Patch

diff --git a/mercurial/extensions.py b/mercurial/extensions.py
--- a/mercurial/extensions.py
+++ b/mercurial/extensions.py
@@ -152,7 +152,7 @@  def afterloaded(extension, callback):
     else:
         _aftercallbacks.setdefault(extension, []).append(callback)
 
-def wrapcommand(table, command, wrapper):
+def wrapcommand(table, command, wrapper, synopsis=None):
     '''Wrap the command named `command' in table
 
     Replace command in the command table with wrapper. The wrapped command will
@@ -182,6 +182,8 @@  def wrapcommand(table, command, wrapper)
 
     newentry = list(entry)
     newentry[0] = wrap
+    if synopsis is not None:
+        newentry[2] = synopsis
     table[key] = tuple(newentry)
     return entry