Patchwork forget: add --dry-run mode

login
register
mail settings
Submitter sushil khanchi
Date March 10, 2018, 7:16 a.m.
Message ID <430c6b5123ee72d3a209.1520666201@workspace>
Download mbox | patch
Permalink /patch/29239/
State Superseded
Headers show

Comments

sushil khanchi - March 10, 2018, 7:16 a.m.
# HG changeset patch
# User Sushil khanchi <sushilkhanchi97@gmail.com>
# Date 1520665399 -19800
#      Sat Mar 10 12:33:19 2018 +0530
# Node ID 430c6b5123ee72d3a209882495302e43b26cc988
# Parent  4c71a26a4009d88590c9ae3d64a5912fd556d82e
forget: add --dry-run mode
Anton Shestakov - March 10, 2018, 7:42 a.m.
On Sat, 10 Mar 2018 12:46: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 430c6b5123ee72d3a209882495302e43b26cc988
> # Parent  4c71a26a4009d88590c9ae3d64a5912fd556d82e
> forget: add --dry-run mode
> 
> diff -r 4c71a26a4009 -r 430c6b5123ee 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, **opts):
>      join = lambda f: os.path.join(prefix, f)
>      bad = []
>      badfn = lambda x, y: bad.append(x) or match.bad(x, y)
> @@ -2039,9 +2039,10 @@
>          if ui.verbose or not match.exact(f):
>              ui.status(_('removing %s\n') % match.rel(f))
>  
> -    rejected = wctx.forget(forget, prefix)
> -    bad.extend(f for f in rejected if f in match.files())
> -    forgot.extend(f for f in forget if f not in rejected)
> +    if not opts.get('dry_run'):

You want to add r to the string, similar to other code in cmdutil that
handles opts. It's a python3-compatibility thing.

(Maybe https://www.mercurial-scm.org/wiki/Python3 needs to be expanded
to mention it explicitly)

> +        rejected = wctx.forget(forget, prefix)
> +        bad.extend(f for f in rejected if f in match.files())
> +        forgot.extend(f for f in forget if f not in rejected)

Does this mean that `bad` is not .extend()ed with --dry-run? It's
checked in commands.forget() to determine exit code. Do we want to have
the same exit code with or without --dry-run? I think we do.

>      return bad, forgot
>  
>  def files(ui, ctx, m, fm, fmt, subrepos):
> diff -r 4c71a26a4009 -r 430c6b5123ee 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',
> +    [('', 'dry-run', None, _('only print output'))]
> +    + walkopts, _('[OPTION]... FILE...'), inferrepo=True)

Having _(...) and inferrepo on separate lines would be more consistent
with other commands.

>  def forget(ui, repo, *pats, **opts):
>      """forget the specified files on the next commit
>  
> @@ -2071,7 +2074,7 @@
>          raise error.Abort(_('no files specified'))
>  
>      m = scmutil.match(repo[None], pats, opts)
> -    rejected = cmdutil.forget(ui, repo, m, prefix="", explicitonly=False)[0]
> +    rejected = cmdutil.forget(ui, repo, m, prefix="", explicitonly=False, **opts)[0]
>      return rejected and 1 or 0
>  
>  @command(
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Anton Shestakov - March 10, 2018, 7:53 a.m.
On Sat, 10 Mar 2018 15:42:04 +0800
Anton Shestakov <av6@dwimlabs.net> wrote:

> On Sat, 10 Mar 2018 12:46: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 430c6b5123ee72d3a209882495302e43b26cc988
> > # Parent  4c71a26a4009d88590c9ae3d64a5912fd556d82e
> > forget: add --dry-run mode
> > 
> > diff -r 4c71a26a4009 -r 430c6b5123ee 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, **opts):
> >      join = lambda f: os.path.join(prefix, f)
> >      bad = []
> >      badfn = lambda x, y: bad.append(x) or match.bad(x, y)
> > @@ -2039,9 +2039,10 @@
> >          if ui.verbose or not match.exact(f):
> >              ui.status(_('removing %s\n') % match.rel(f))
> >  
> > -    rejected = wctx.forget(forget, prefix)
> > -    bad.extend(f for f in rejected if f in match.files())
> > -    forgot.extend(f for f in forget if f not in rejected)
> > +    if not opts.get('dry_run'):
> 
> You want to add r to the string, similar to other code in cmdutil that
> handles opts.

Looks like only add() is doing this so far:
https://www.mercurial-scm.org/repo/hg/rev/a77e61b45384
But it still looks like you want r'' here.

Patch

diff -r 4c71a26a4009 -r 430c6b5123ee 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, **opts):
     join = lambda f: os.path.join(prefix, f)
     bad = []
     badfn = lambda x, y: bad.append(x) or match.bad(x, y)
@@ -2039,9 +2039,10 @@ 
         if ui.verbose or not match.exact(f):
             ui.status(_('removing %s\n') % match.rel(f))
 
-    rejected = wctx.forget(forget, prefix)
-    bad.extend(f for f in rejected if f in match.files())
-    forgot.extend(f for f in forget if f not in rejected)
+    if not opts.get('dry_run'):
+        rejected = wctx.forget(forget, prefix)
+        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
 
 def files(ui, ctx, m, fm, fmt, subrepos):
diff -r 4c71a26a4009 -r 430c6b5123ee 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',
+    [('', 'dry-run', None, _('only print output'))]
+    + walkopts, _('[OPTION]... FILE...'), inferrepo=True)
 def forget(ui, repo, *pats, **opts):
     """forget the specified files on the next commit
 
@@ -2071,7 +2074,7 @@ 
         raise error.Abort(_('no files specified'))
 
     m = scmutil.match(repo[None], pats, opts)
-    rejected = cmdutil.forget(ui, repo, m, prefix="", explicitonly=False)[0]
+    rejected = cmdutil.forget(ui, repo, m, prefix="", explicitonly=False, **opts)[0]
     return rejected and 1 or 0
 
 @command(