Patchwork [V4] forget: add --dry-run mode

# HG changeset patch
# User Sushil khanchi <sushilkhanchi97@gmail.com>
# Date 1520665399 -19800
#      Sat Mar 10 12:33:19 2018 +0530
# Node ID 4134d1f103d8e72e80920cd89cf3d3f576e6a2b5
# Parent  4c71a26a4009d88590c9ae3d64a5912fd556d82e
forget: add --dry-run mode

On Tue, 13 Mar 2018 12:17:41 +0530
Sushil khanchi <sushilkhanchi97@gmail.com> wrote:

> # HG changeset patch
> # User Sushil khanchi <sushilkhanchi97@gmail.com>
> # Date 1520665399 -19800
> #      Sat Mar 10 12:33:19 2018 +0530
> # Node ID 4134d1f103d8e72e80920cd89cf3d3f576e6a2b5
> # Parent  4c71a26a4009d88590c9ae3d64a5912fd556d82e
> forget: add --dry-run mode
> 
> diff -r 4c71a26a4009 -r 4134d1f103d8 mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py	Sun Mar 04 21:16:36 2018 -0500
> +++ b/mercurial/cmdutil.py	Sat Mar 10 12:33:19 2018 +0530
> @@ -1996,7 +1996,7 @@
>          for subpath in ctx.substate:
>              ctx.sub(subpath).addwebdirpath(serverpath, webconf)
>  
> -def forget(ui, repo, match, prefix, explicitonly):
> +def forget(ui, repo, match, prefix, explicitonly, dryrun):
>      join = lambda f: os.path.join(prefix, f)
>      bad = []
>      badfn = lambda x, y: bad.append(x) or match.bad(x, y)
> @@ -2012,7 +2012,7 @@
>          sub = wctx.sub(subpath)
>          try:
>              submatch = matchmod.subdirmatcher(subpath, match)
> -            subbad, subforgot = sub.forget(submatch, prefix)
> +            subbad, subforgot = sub.forget(submatch, prefix, dryrun=dryrun)
>              bad.extend([subpath + '/' + f for f in subbad])
>              forgot.extend([subpath + '/' + f for f in subforgot])
>          except error.LookupError:
> @@ -2039,7 +2039,7 @@
>          if ui.verbose or not match.exact(f):
>              ui.status(_('removing %s\n') % match.rel(f))
>  
> -    rejected = wctx.forget(forget, prefix)
> +    rejected = wctx.forget(forget, prefix, dryrun=dryrun)
>      bad.extend(f for f in rejected if f in match.files())
>      forgot.extend(f for f in forget if f not in rejected)
>      return bad, forgot
> diff -r 4c71a26a4009 -r 4134d1f103d8 mercurial/commands.py
> --- a/mercurial/commands.py	Sun Mar 04 21:16:36 2018 -0500
> +++ b/mercurial/commands.py	Sat Mar 10 12:33:19 2018 +0530
> @@ -2036,7 +2036,10 @@
>      with ui.formatter('files', opts) as fm:
>          return cmdutil.files(ui, ctx, m, fm, fmt, opts.get('subrepos'))
>  
> -@command('^forget', walkopts, _('[OPTION]... FILE...'), inferrepo=True)
> +@command(
> +    '^forget',
> +    walkopts + dryrunopts,
> +    _('[OPTION]... FILE...'), inferrepo=True)
>  def forget(ui, repo, *pats, **opts):
>      """forget the specified files on the next commit
>  
> @@ -2071,7 +2074,8 @@
>          raise error.Abort(_('no files specified'))
>  
>      m = scmutil.match(repo[None], pats, opts)
> -    rejected = cmdutil.forget(ui, repo, m, prefix="", explicitonly=False)[0]
> +    dryrun = opts.get(r'dry_run')
> +    rejected = cmdutil.forget(ui, repo, m, prefix="", explicitonly=False, dryrun=dryrun)[0]

This line is too long, don't forget to also run test-check-commit.t.

>      return rejected and 1 or 0
>  
>  @command(
> diff -r 4c71a26a4009 -r 4134d1f103d8 mercurial/context.py
> --- a/mercurial/context.py	Sun Mar 04 21:16:36 2018 -0500
> +++ b/mercurial/context.py	Sat Mar 10 12:33:19 2018 +0530
> @@ -1564,7 +1564,7 @@
>                      ds.add(f)
>              return rejected
>  
> -    def forget(self, files, prefix=""):
> +    def forget(self, files, prefix="", dryrun=False):
>          with self._repo.wlock():
>              ds = self._repo.dirstate
>              uipath = lambda f: ds.pathto(pathutil.join(prefix, f))
> @@ -1573,10 +1573,11 @@
>                  if f not in self._repo.dirstate:
>                      self._repo.ui.warn(_("%s not tracked!\n") % uipath(f))
>                      rejected.append(f)
> -                elif self._repo.dirstate[f] != 'a':
> -                    self._repo.dirstate.remove(f)
> -                else:
> -                    self._repo.dirstate.drop(f)
> +                elif not dryrun:
> +                    if self._repo.dirstate[f] != 'a':
> +                        self._repo.dirstate.remove(f)
> +                    else:
> +                        self._repo.dirstate.drop(f)
>              return rejected
>  
>      def undelete(self, list):
> diff -r 4c71a26a4009 -r 4134d1f103d8 mercurial/subrepo.py
> --- a/mercurial/subrepo.py	Sun Mar 04 21:16:36 2018 -0500
> +++ b/mercurial/subrepo.py	Sat Mar 10 12:33:19 2018 +0530
> @@ -811,9 +811,10 @@
>          return ctx.walk(match)
>  
>      @annotatesubrepoerror
> -    def forget(self, match, prefix):
> +    def forget(self, match, prefix, dryrun):
>          return cmdutil.forget(self.ui, self._repo, match,
> -                              self.wvfs.reljoin(prefix, self._path), True)
> +                              self.wvfs.reljoin(prefix, self._path),
> +                              True, dryrun=dryrun)
>  
>      @annotatesubrepoerror
>      def removefiles(self, matcher, prefix, after, force, subrepos, warnings):
> diff -r 4c71a26a4009 -r 4134d1f103d8 tests/test-commit.t
> --- a/tests/test-commit.t	Sun Mar 04 21:16:36 2018 -0500
> +++ b/tests/test-commit.t	Sat Mar 10 12:33:19 2018 +0530
> @@ -832,3 +832,15 @@
>  
>    $ cd ..
>  
> +# test --dry-run mode in $hg forget

Test name is not a comment, so it shouldn't start with a #. Putting $
before hg is not a convention we have, and the whole "$hg" here can be
dropped altogether because it's implicitly understood (we're in
Mercurial test suite).

> +
> +  $ hg init testdir_forget
> +  $ cd testdir_forget
> +  $ echo foo > foo
> +  $ hg add foo
> +  $ hg commit -m "foo"
> +  $ hg forget foo --dry-run -v
> +  removing foo
> +  $ hg diff

Since it's an important feature of --dry-run to preserve exit code,
let's test what happens if we try to `hg forget -n` a non-existing
file, for example.

> +
> +  $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

On Tue, 13 Mar 2018 15:43:02 +0800
Anton Shestakov <av6@dwimlabs.net> wrote:

> > +    rejected = cmdutil.forget(ui, repo, m, prefix="", explicitonly=False, dryrun=dryrun)[0]  
> 
> This line is too long, don't forget to also run test-check-commit.t.

I saw this already fixed, but I'd like to get this right.

test-check-code.t is the test that would detect lines of code being too
long (not *-commit.t), but running all of the test-check-*.t before
sending patches is a good idea. Small issues can be fixed when
reviewers queue patches, but not all of check-code.py (as an example)
is style checks; some of the rules are important for compatibility
(Python 2 vs 3, and running test suite on different OSes), and they are
put there because they can be tricky to spot during manual review.