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.

INLINE COMMENTS

> uncommit.py:140
>      [('', '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.

REPOSITORY
  rHG Mercurial

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

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


More information about the Mercurial-devel mailing list