[PATCH 2 of 2] wireproto: move wireproto capabilities computation in a subfunction

[David Soria Parra]

pierre-yves.david at ens-lyon.org writes:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at fb.com>
> # Date 1394660801 25200
> #      Wed Mar 12 14:46:41 2014 -0700
> # Node ID ccf1552655b90cac5157e86fc914a0a33c46ea24
> # Parent  1bfa5d82da6540e6559c3396c692db848ea650f2
> wireproto: move wireproto capabilities computation in a subfunction
>
> It will help people that need to add capabilities (in a more subtle was that
> just adding some to the list) in multiple way:
>
> 1. This function returns a list, not a string. Making it easier to look at,
>    extend or alter the content.
>
> 2. The original capabilities function will be store in the dictionary of wire
>    protocol command. So extension that wrap this function also need to update
>    the dictionary entry.
>
>    Both wrapping and update of the dictionary entry are needed because the
>    `hello` wire protocol use the function itself. This is specifically sneaky for
>    extension writer as ssh use the `hello` command while http use the
>    `capabilities` command.
>
>    With this new `_capabilities` function there is one and only one obvious
>    place to wrap when needed.
>
> diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
> --- a/mercurial/wireproto.py
> +++ b/mercurial/wireproto.py
> @@ -415,11 +415,20 @@ def branches(repo, proto, nodes):
>      return "".join(r)
>  
>  
>  WIREPROTOCAPS = ['lookup', 'changegroupsubset', 'branchmap', 'pushkey',
>                   'known', 'getbundle', 'unbundlehash', 'batch']
> -def capabilities(repo, proto):
> +
> +def _capabilities(repo, proto):
> +    """return a list of capabilities for a repo
> +
> +    This function exist to easy wrapping process from extension
Maybe it's because I am not a native speaker but I honestly have a hard
time understanding what you mean.

> +    - returns a lists: easy to alter
> +    - change done here will be progated to both `capabilities` and `hello`
> +      command without any other effort. without any other action needed.
> +    """
my dictionary doesn't find "progated".
>      # copy to prevent modification of the global list
>      caps = list(WIREPROTOCAPS)
>      if _allowstream(repo.ui):
>          if repo.ui.configbool('server', 'preferuncompressed', False):
>              caps.append('stream-preferred')
> @@ -430,10 +439,16 @@ def capabilities(repo, proto):
>          # otherwise, add 'streamreqs' detailing our local revlog format
>          else:
>              caps.append('streamreqs=%s' % ','.join(requiredformats))
>      caps.append('unbundle=%s' % ','.join(changegroupmod.bundlepriority))
>      caps.append('httpheader=1024')
> +    return caps
> +
> +# If you are writting and extension and consider wrapping this function. Wrap
> +# `_capabilities` instead.
> +def capabilities(repo, proto):
> +    caps = _capabilities(repo, proto)
why not just return ' '.join(_capabilities(repo,proto)) ?

besodes the nits it looks good to me.