Re: [PATCH V2] forget: add --dry-run mode
On Tue, 13 Mar 2018 18:51:33 -0400, Matt Harbison wrote: > On Tue, 13 Mar 2018 17:30:55 -0400, sushil khanchi > 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 Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH V2] forget: add --dry-run mode
On Tue, 13 Mar 2018 17:30:55 -0400, sushil khanchi 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 Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH V2] forget: add --dry-run mode
Shall I remove --dry-run from context layer now?
On Tue, Mar 13, 2018 at 7:55 PM, Yuya Nishihara wrote:
> On Mon, 12 Mar 2018 18:27:13 -0400, Matt Harbison wrote:
> > On Sun, 11 Mar 2018 05:49:26 -0400, Sushil khanchi
> > wrote:
> >
> > > # HG changeset patch
> > > # User Sushil khanchi
> > > # 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
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH V2] forget: add --dry-run mode
On Mon, 12 Mar 2018 18:27:13 -0400, Matt Harbison wrote:
> On Sun, 11 Mar 2018 05:49:26 -0400, Sushil khanchi
> wrote:
>
> > # HG changeset patch
> > # User Sushil khanchi
> > # 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
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH V2] forget: add --dry-run mode
On Sun, 11 Mar 2018 05:49:26 -0400, Sushil khanchi
wrote:
# HG changeset patch
# User Sushil khanchi
# 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
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH V2] forget: add --dry-run mode
On Sun, 11 Mar 2018 15:19:26 +0530, Sushil khanchi wrote:
> # HG changeset patch
> # User Sushil khanchi
> # 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
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
