[PATCH V3] uncommit: abort if an explicitly given file cannot be uncommitted

[Yuya Nishihara]

On Sun, 31 Mar 2019 22:46:25 -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison at yahoo.com>
> # Date 1553910795 14400
> #      Fri Mar 29 21:53:15 2019 -0400
> # Node ID 9802203e693d83f4092512be6d3e397926b36f8e
> # Parent  eec20025ada33889233e553c5825aac36b708f6c
> uncommit: abort if an explicitly given file cannot be uncommitted
> 
> I've gotten burned several times by this in the last few days.  The former tests
> look simple enough, but if a good file and a bad file are given, the bad files
> are silently ignored.  Some commands like `forget` will warn about bogus files,
> but that would likely get lost in the noise of an interactive uncommit.  The
> commit command aborts if a bad file is given, so this seems more consistent for
> commands that alter the repository.
> 
> diff --git a/hgext/uncommit.py b/hgext/uncommit.py
> --- a/hgext/uncommit.py
> +++ b/hgext/uncommit.py
> @@ -133,8 +133,36 @@ def uncommit(ui, repo, *pats, **opts):
>          if len(old.parents()) > 1:
>              raise error.Abort(_("cannot uncommit merge changeset"))
>  
> +        match = scmutil.match(old, pats, opts)
> +
> +        # Check all explicitly given files; abort if there's a problem.
> +        if match.files():
> +            s = old.status(old.p1(), match, listclean=True)
> +            eligible = set(s.added) | set(s.modified) | set(s.removed)
> +
> +            for f in match.files():
> +                if f not in eligible:
> +                    # Naming a parent directory of an eligible file is OK, even
> +                    # if not everything tracked in that directory can be
> +                    # uncommitted.
> +                    for e in eligible:
> +                        if e.startswith(f + '/'):
> +                            break

Perhaps, the eligible set can be extended to include util.dirs(eligible)
to get around possible quadratic computation.

That said, do we really want to error out if <dir> doesn't match any modified
files? If I do "hg <whatever> <dir>", I don't care if <dir> is empty or not.
I expect it'll be basically the same as "cd <dir> && hg <whatever> .".

> +                    else:
> +                        if f in s.clean:
> +                            hint = _(b"file was not changed in working "
> +                                     b"directory parent")
> +                        elif repo.wvfs.exists(f):
> +                            hint = _(b"file was untracked in working directory "
> +                                     b"parent")
> +                        else:
> +                            hint = _(b"file does not exist")
> +
> +                        raise error.Abort(_(b'cannot uncommit "%s"')
> +                                          % scmutil.getuipathfn(repo)(f),
> +                                          hint=hint)