[PATCH V2] histedit: pick an appropriate base changeset by default (BC)

Pierre-Yves David pierre-yves.david at ens-lyon.org
Sun Dec 6 02:46:44 CST 2015



On 12/03/2015 10:30 PM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # Date 1445712999 -3600
> #      Sat Oct 24 19:56:39 2015 +0100
> # Node ID e622d985cf66f761b4873d846bae4ebe77a8a471
> # Parent  71aa5a26162d6e4a165b68f07b331e3e2eedc117
> histedit: pick an appropriate base changeset by default (BC)
>
> Previously, `hg histedit` required a revision argument specifying which
> revision to use as the base for the current histedit operation. There
> was an undocumented and experimental "histedit.defaultrev" option that
> supported defining a single revision to be used if no argument is
> passed.

I'm considered bikesheding the config name (to move that in a config 
section dedicated to default destination). However, we already have an 
histedit section (for another debatable option). So lets shove in that 
histedit section, we can still deprecate the whole thing if we come with 
a good standard definition for "your stack".

[...]
> --- a/mercurial/destutil.py
> +++ b/mercurial/destutil.py
> @@ -193,8 +193,20 @@ def _destmergebranch(repo):
>       return node
>
>   def destmerge(repo):
>       if repo._activebookmark:
>           node = _destmergebook(repo)
>       else:
>           node = _destmergebranch(repo)
>       return repo[node].rev()
> +
> +def desthistedit(ui, repo):
> +    """Default base revision to edit for `hg histedit`."""
> +    default = ui.config('histedit', 'defaultrev',
> +            'limit(reverse(only(.) and not public() and not ::merge()), 50)')

Lets put the default in a temporary variable to avoid the huge line wrap.

While we are at it, lets put it as a top level constant to help wrapper 
who would like to change just that.

> +    if default:
> +        revs = repo.revs(default)
> +        if revs:
> +            revs.sort()
> +            return revs.first()

I a got a 5 seconds puzzling about why this you were sorting an already 
sorted revset and why the 'first()' was not part of the revset itself.

While you code is correct (as user config are not guaranteed to have the 
sorting and first), you should probably add a comment about it to make 
sure a future reader do not miss that fact.

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list