Patchwork D6069: uncommit: add flag --allow-dirty-working-copy

login
register
mail settings
Submitter phabricator
Date March 4, 2019, 3:04 p.m.
Message ID <differential-rev-PHID-DREV-qojyvczdkbxn5dlxetjg-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/39046/
State Superseded
Headers show

Comments

phabricator - March 4, 2019, 3:04 p.m.
navaneeth.suresh created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This adds a flag `--allow-dirty-working-copy` as an alias for
  the experimental config option `experimental.uncommitondirtydir`.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6069

AFFECTED FILES
  hgext/uncommit.py
  tests/test-uncommit.t

CHANGE DETAILS




To: navaneeth.suresh, #hg-reviewers
Cc: mercurial-devel
phabricator - March 4, 2019, 3:14 p.m.
pulkit added inline comments.

INLINE COMMENTS

> uncommit.py:162
>          isdirtypath = any(set(m + a + r + d) & set(pats))
> +        allowdirtywcopy = opts['allow_dirty_working_copy']
>          if not repo.ui.configbool('experimental', 'uncommitondirtywdir') and \

how about making this as:

`allowdirty = opts['allow_dirty_working_copy'] or repo.ui.configbool('experimental', 'uncommitondirtywdir')`

This will make the if condition more simpler to understand.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6069

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - March 4, 2019, 3:40 p.m.
navaneeth.suresh added inline comments.

INLINE COMMENTS

> pulkit wrote in uncommit.py:162
> how about making this as:
> 
> `allowdirty = opts['allow_dirty_working_copy'] or repo.ui.configbool('experimental', 'uncommitondirtywdir')`
> 
> This will make the if condition more simpler to understand.

Done.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6069

To: navaneeth.suresh, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - March 7, 2019, 5:38 p.m.
mharbison72 added inline comments.

INLINE COMMENTS

> test-uncommit.t:164
>    abort: uncommitted changes
> -  (requires experimental.uncommitondirtywdir to uncommit)
> +  (requires --allow-dirty-working-copy to uncommit)
>    [255]

Did we lose test coverage here?  The output before the prior patch noted it was keeping an empty commit, so maybe the suggested flag needs to be used after this abort?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6069

To: navaneeth.suresh, #hg-reviewers, pulkit
Cc: mharbison72, pulkit, mercurial-devel
phabricator - March 7, 2019, 7:51 p.m.
navaneeth.suresh added inline comments.

INLINE COMMENTS

> mharbison72 wrote in test-uncommit.t:164
> Did we lose test coverage here?  The output before the prior patch noted it was keeping an empty commit, so maybe the suggested flag needs to be used after this abort?

I'm not quite sure about this. I'll wait for @pulkit. Also, the fix won't be that easy if needed.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6069

To: navaneeth.suresh, #hg-reviewers, pulkit
Cc: mharbison72, pulkit, mercurial-devel
phabricator - March 8, 2019, 6:01 p.m.
pulkit added inline comments.

INLINE COMMENTS

> mharbison72 wrote in test-uncommit.t:164
> Did we lose test coverage here?  The output before the prior patch noted it was keeping an empty commit, so maybe the suggested flag needs to be used after this abort?

@mharbison72 I looked at the test file an there are more instances of that being tested. I am not sure whether we test that here also.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6069

To: navaneeth.suresh, #hg-reviewers, pulkit
Cc: mharbison72, pulkit, mercurial-devel

Patch

diff --git a/tests/test-uncommit.t b/tests/test-uncommit.t
--- a/tests/test-uncommit.t
+++ b/tests/test-uncommit.t
@@ -34,9 +34,10 @@ 
   
   options ([+] can be repeated):
   
-      --keep                allow an empty commit after uncommiting
-   -I --include PATTERN [+] include names matching the given patterns
-   -X --exclude PATTERN [+] exclude names matching the given patterns
+      --keep                     allow an empty commit after uncommiting
+      --allow-dirty-working-copy allow uncommit with outstanding changes
+   -I --include PATTERN [+]      include names matching the given patterns
+   -X --exclude PATTERN [+]      exclude names matching the given patterns
   
   (some details hidden, use --verbose to show complete help)
 
@@ -156,11 +157,11 @@ 
   M files
   $ hg uncommit
   abort: uncommitted changes
-  (requires experimental.uncommitondirtywdir to uncommit)
+  (requires --allow-dirty-working-copy to uncommit)
   [255]
   $ hg uncommit files
   abort: uncommitted changes
-  (requires experimental.uncommitondirtywdir to uncommit)
+  (requires --allow-dirty-working-copy to uncommit)
   [255]
   $ cat files
   abcde
@@ -172,7 +173,7 @@ 
   $ echo "bar" >> files
   $ hg uncommit
   abort: uncommitted changes
-  (requires experimental.uncommitondirtywdir to uncommit)
+  (requires --allow-dirty-working-copy to uncommit)
   [255]
   $ hg uncommit --config experimental.uncommitondirtywdir=True
   $ hg commit -m "files abcde + foo"
@@ -371,7 +372,7 @@ 
 
   $ hg uncommit
   abort: outstanding uncommitted merge
-  (requires experimental.uncommitondirtywdir to uncommit)
+  (requires --allow-dirty-working-copy to uncommit)
   [255]
 
   $ hg uncommit --config experimental.uncommitondirtywdir=True
@@ -418,7 +419,7 @@ 
   $ hg unc a
   $ hg unc b
   abort: uncommitted changes
-  (requires experimental.uncommitondirtywdir to uncommit)
+  (requires --allow-dirty-working-copy to uncommit)
   [255]
   $ cat a
   super critical info!
@@ -432,7 +433,7 @@ 
   
   $ hg ci -Am 'add b'
   $ echo 'foo bar' > b
-  $ hg unc --config experimental.uncommitondirtywdir=True b
+  $ hg unc --allow-dirty-working-copy b
   $ hg log
   changeset:   3:30fa958635b2
   tag:         tip
diff --git a/hgext/uncommit.py b/hgext/uncommit.py
--- a/hgext/uncommit.py
+++ b/hgext/uncommit.py
@@ -137,6 +137,8 @@ 
 
 @command('uncommit',
     [('', 'keep', False, _('allow an empty commit after uncommiting')),
+     ('', 'allow-dirty-working-copy', False,
+    _('allow uncommit with outstanding changes'))
     ] + commands.walkopts,
     _('[OPTION]... [FILE]...'),
     helpcategory=command.CATEGORY_CHANGE_MANAGEMENT)
@@ -157,10 +159,11 @@ 
 
         m, a, r, d = repo.status()[:4]
         isdirtypath = any(set(m + a + r + d) & set(pats))
+        allowdirtywcopy = opts['allow_dirty_working_copy']
         if not repo.ui.configbool('experimental', 'uncommitondirtywdir') and \
-            (not pats or isdirtypath):
+            (not pats or isdirtypath and not allowdirtywcopy):
             cmdutil.bailifchanged(repo, hint=_('requires '
-                                'experimental.uncommitondirtywdir to uncommit'))
+                                '--allow-dirty-working-copy to uncommit'))
         old = repo['.']
         rewriteutil.precheck(repo, [old.rev()], 'uncommit')
         if len(old.parents()) > 1: