[PATCH V2] forget: add --dry-run mode

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH V2] forget: add --dry-run mode

sushil khanchi
# HG changeset patch
# User Sushil khanchi <[hidden email]>
# Date 1520665399 -19800
#      Sat Mar 10 12:33:19 2018 +0530
# Node ID a1be8989c0158abc69ebd97ca8a0cc7dc3801be9
# Parent  4c71a26a4009d88590c9ae3d64a5912fd556d82e
forget: add --dry-run mode

diff -r 4c71a26a4009 -r a1be8989c015 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(r'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 a1be8989c015 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,11 @@
     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 +2075,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
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH V2] forget: add --dry-run mode

Yuya Nishihara
On Sun, 11 Mar 2018 15:19:26 +0530, Sushil khanchi wrote:
> # HG changeset patch
> # User Sushil khanchi <[hidden email]>
> # Date 1520665399 -19800
> #      Sat Mar 10 12:33:19 2018 +0530
> # Node ID a1be8989c0158abc69ebd97ca8a0cc7dc3801be9
> # Parent  4c71a26a4009d88590c9ae3d64a5912fd556d82e
> forget: add --dry-run mode

Can you add some tests?

> -def forget(ui, repo, match, prefix, explicitonly):
> +def forget(ui, repo, match, prefix, explicitonly, **opts):

I slightly prefer an explicit parameter, 'opts' or 'dryrun' instead of **opts.
That will show you we'll have to propagate the option to sub.forget().

> -    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(r'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 a1be8989c015 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,11 @@
>      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,

The constant 'dryrunopts' can be used instead.
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH V2] forget: add --dry-run mode

Matt Harbison-2
In reply to this post by sushil khanchi
On Sun, 11 Mar 2018 05:49:26 -0400, Sushil khanchi  
<[hidden email]> wrote:

> # HG changeset patch
> # User Sushil khanchi <[hidden email]>
> # Date 1520665399 -19800
> #      Sat Mar 10 12:33:19 2018 +0530
> # Node ID a1be8989c0158abc69ebd97ca8a0cc7dc3801be9
> # Parent  4c71a26a4009d88590c9ae3d64a5912fd556d82e
> forget: add --dry-run mode
>
> diff -r 4c71a26a4009 -r a1be8989c015 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)

Shouldn't --dry-run be passed into wctx.forget() too?  Then the warning  
about bad paths there will be emitted, and you won't have to  
conditionalize the following lines here.  That in turn won't affect the  
exit code.

> -    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(r'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 a1be8989c015 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,11 @@
>      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 +2075,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
> [hidden email]
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH V2] forget: add --dry-run mode

Yuya Nishihara
On Mon, 12 Mar 2018 18:27:13 -0400, Matt Harbison wrote:

> On Sun, 11 Mar 2018 05:49:26 -0400, Sushil khanchi  
> <[hidden email]> wrote:
>
> > # HG changeset patch
> > # User Sushil khanchi <[hidden email]>
> > # Date 1520665399 -19800
> > #      Sat Mar 10 12:33:19 2018 +0530
> > # Node ID a1be8989c0158abc69ebd97ca8a0cc7dc3801be9
> > # Parent  4c71a26a4009d88590c9ae3d64a5912fd556d82e
> > forget: add --dry-run mode
> >
> > diff -r 4c71a26a4009 -r a1be8989c015 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)
>
> Shouldn't --dry-run be passed into wctx.forget() too?  Then the warning  
> about bad paths there will be emitted, and you won't have to  
> conditionalize the following lines here.  That in turn won't affect the  
> exit code.

I slightly prefer not to pass --dry-run deep into the context layer
because it's a command-line business. Just my two cents.
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH V2] forget: add --dry-run mode

sushil khanchi
Shall I remove --dry-run from context layer now?

On Tue, Mar 13, 2018 at 7:55 PM, Yuya Nishihara <[hidden email]> wrote:
On Mon, 12 Mar 2018 18:27:13 -0400, Matt Harbison wrote:
> On Sun, 11 Mar 2018 05:49:26 -0400, Sushil khanchi
> <[hidden email]> wrote:
>
> > # HG changeset patch
> > # User Sushil khanchi <[hidden email]>
> > # Date 1520665399 -19800
> > #      Sat Mar 10 12:33:19 2018 +0530
> > # Node ID a1be8989c0158abc69ebd97ca8a0cc7dc3801be9
> > # Parent  4c71a26a4009d88590c9ae3d64a5912fd556d82e
> > forget: add --dry-run mode
> >
> > diff -r 4c71a26a4009 -r a1be8989c015 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)
>
> Shouldn't --dry-run be passed into wctx.forget() too?  Then the warning
> about bad paths there will be emitted, and you won't have to
> conditionalize the following lines here.  That in turn won't affect the
> exit code.

I slightly prefer not to pass --dry-run deep into the context layer
because it's a command-line business. Just my two cents.


_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH V2] forget: add --dry-run mode

Matt Harbison-2
On Tue, 13 Mar 2018 17:30:55 -0400, sushil khanchi  
<[hidden email]> wrote:

> Shall I remove --dry-run from context layer now?

It looks like cmdutil.add() and scmutil.addremove() don't pass it along to  
wctx either, so I guess it's OK to drop for consistency.

But the fact that it doesn't seems like a bug- wctx.add() prints out  
various warnings about portable file names, sizes, etc.  I'd think that  
the whole point of a dry run is to test against that.  Maybe the solution  
to that is to have a mock wctx somehow that simply doesn't flush dirstate  
to disk?
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH V2] forget: add --dry-run mode

Yuya Nishihara
On Tue, 13 Mar 2018 18:51:33 -0400, Matt Harbison wrote:

> On Tue, 13 Mar 2018 17:30:55 -0400, sushil khanchi  
> <[hidden email]> wrote:
>
> > Shall I remove --dry-run from context layer now?
>
> It looks like cmdutil.add() and scmutil.addremove() don't pass it along to  
> wctx either, so I guess it's OK to drop for consistency.
>
> But the fact that it doesn't seems like a bug- wctx.add() prints out  
> various warnings about portable file names, sizes, etc.  I'd think that  
> the whole point of a dry run is to test against that.  Maybe the solution  
> to that is to have a mock wctx somehow that simply doesn't flush dirstate  
> to disk?

That could be, but I think the check for bad paths is actually done by
cmdutil.forget(). Because ctx.forget() doesn't take a matcher, the caller
has to resolve patterns against dirstate.
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel