D5940: uncommit: add -f/--force when possibly hiding data (issue5977)
martinvonz (Martin von Zweigbergk)
phabricator at mercurial-scm.org
Fri Feb 15 12:36:02 EST 2019
martinvonz added inline comments.
> [('', 'keep', False, _('allow an empty commit after uncommiting')),
> + ('f', 'force', False, _('allow uncommit with outstanding changes')),
> ] + commands.walkopts,
Could you rename --force to e.g. --allow-dirty-working-copy so it's clearer what's being allowed?
> navaneeth.suresh wrote in test-uncommit.t:176-179
> I'm also surprised at the change in that output. Do you recommend removing that config option if that doesn't do anything new?
I like that config option and I don't want to lose it. I was even hoping it would be on by default some day.
IIUC, the new behavior is to require --force if there are uncommitted changes, whether you're uncommitting one file or all. I was actually surprised that it used to be allowed to uncommit specific files on a dirty working copy. I like the new consistency more.
I think the --force option should be equivalent to the existing config. How about we just add `and not opts.get('force')` to the existing condition on line 159? I'd also prefer to drop the `not pats` from that condition, but that can (and should) be in a separate patch.
To: navaneeth.suresh, #hg-reviewers
Cc: martinvonz, pulkit, mercurial-devel
More information about the Mercurial-devel