[PATCH 4 of 6] resolve: abort when not applicable

[Mads Kiilerich]

On 05/09/2014 03:46 AM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # Date 1397873312 25200
> #      Fri Apr 18 19:08:32 2014 -0700
> # Node ID a5e623de6557ea8854cefc108d5b069adb0f737a
> # Parent  6d5b402aae2d697b93b19ccf8f4d0245c4fc29d3
> resolve: abort when not applicable
>
> The resolve command is only relevant when mergestate is present.
> This patch will make resolve abort when no mergestate is present.
>
> This change will let people know when they are using resolve when they
> shouldn't be. This change will let people know when their use of resolve
> doesn't do anything.
>
> Previously, |hg resolve -m| would allow mergestate to be created. This
> patch now forbids that. Strictly speaking, this is backwards
> incompatible. The author of this patch believes creating mergestate via
> resolve doesn't make much sense and this side-effect was unintended.
>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -4926,8 +4926,12 @@ def resolve(ui, repo, *pats, **opts):
>           raise util.Abort(_('no files or directories specified; '
>                              'use --all to remerge all files'))
>   
>       ms = mergemod.mergestate(repo)
> +
> +    if not ms.active():
> +        raise util.Abort(_('resolve command not applicable when not merging'))
> +
>       m = scmutil.match(repo[None], pats, opts)
>       ret = 0
>   
>       for f in ms:
> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -175,8 +175,20 @@ class mergestate(object):
>               if err.errno != errno.ENOENT:
>                   raise
>           return records
>   
> +    def active(self):
> +        """Whether mergestate is active.
> +
> +        Returns True if there appears to be mergestate. This is a rough proxy
> +        for "is a merge in progress."
> +        """
> +        # Check local variables before looking at filesystem for performance
> +        # reasons.
> +        return bool(self._local) or bool(self._state) or \
> +               self._repo.opener.exists(self.statepathv1) or \
> +               self._repo.opener.exists(self.statepathv2)

Did you have a response to my question on 
http://selenic.com/pipermail/mercurial-devel/2014-May/058585.html :

Will there ever be a (relevant) case where _local doesn't give the full 
answer? A mergestate without  a _local (and _other) is no mergestate.

It seems to me like this code is more complex than necessary.

/Mads