[PATCH] commitextras: move fb's extension which add extras to a commit to core

[Yuya Nishihara]

On Wed, 05 Jul 2017 23:14:13 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit at gmail.com>
> # Date 1499274297 -19800
> #      Wed Jul 05 22:34:57 2017 +0530
> # Node ID 64c39cf4b61475a133f88fa6dd247ca4f3d61436
> # Parent  8e6f4939a69ae8949e134d97de6b766799bca8de
> # EXP-Topic fbext
> commitextras: move fb's extension which add extras to a commit to core
> 
> Facebook has this as extension which adds a flag to `hg commit` to add extra
> values to a commit from command line. This patch moves that extension to core by
> adding that flag and marking that as ADVANCED. Also this patch adds a function
> to check if any key which is used internally can't be used.

FYI, the last attempt was rejected.

https://www.mercurial-scm.org/pipermail/mercurial-devel/2014-March/056889.html

I don't know if ADVANCED flag can make the "high barrier" enough to discourage
an abuse of extras.

> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -175,6 +175,13 @@
>  def parsealiases(cmd):
>      return cmd.lstrip("^").split("|")
>  
> +def checkextras(extras):
> +    notallowed = set(['amend_source'])

This should include 'branch', at least.

> +    msg = _("key '%s' is used internally, can't be set manually")
> +    for key in extras:
> +        if key in notallowed:
> +            raise error.Abort(msg % key)

Perhaps we should have some constraints on key format, e.g. '',
'non&ascii*string'.

>  def setupwrapcolorwrite(ui):
>      # wrap ui.write so diff output can be labeled/colorized
>      def wrapwrite(orig, *args, **kw):
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -1420,6 +1420,8 @@
>      ('s', 'secret', None, _('use the secret phase for committing')),
>      ('e', 'edit', None, _('invoke editor on commit messages')),
>      ('i', 'interactive', None, _('use interactive mode')),
> +    ('', 'extra', [], _('set a changeset\'s extra values (ADVANCED)'),
> +        _('KEY=VALUE')),
>      ] + walkopts + commitopts + commitopts2 + subrepoopts,
>      _('[OPTION]... [FILE]...'),
>      inferrepo=True)
> @@ -1488,6 +1490,16 @@
>          release(lock, wlock)
>  
>  def _docommit(ui, repo, *pats, **opts):
> +    extrals = opts.get('extra')
> +    extra = {}
> +    # extrals can be none, so checking before iterating on it
> +    if extrals:
> +        for raws in extrals:
> +            k, v = raws.split('=', 1)

Need to check if '=' exists.

> +            extra[k] = v
> +    # check if any extra key is passed which is used internally
> +    cmdutil.checkextras(extra)