| 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
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
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
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
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
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: