This is an archive of the discontinued Mercurial Phabricator instance.

resolve: add confirm config option
ClosedPublic

Authored by khanchi97 on Jul 31 2018, 9:50 AM.

Details

Summary

This config setting gives a functionality to confirm before
it re-merge all unresolved files. If this config is enabled,
when you run 'hg resolve --all' it will prompt with a msg
"re-merge all unresolved files (yn)?"

To enable this functionality:
[commands]
resolve.confirm = True

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

khanchi97 created this revision.Jul 31 2018, 9:50 AM
pulkit added a subscriber: pulkit.Aug 1 2018, 9:06 AM
pulkit added inline comments.
mercurial/commands.py
4537

I missed that we need --all, so make this prompt only when --all is passed.

mercurial/configitems.py
1165

commands.resolve.confirm sounds a better place.

khanchi97 edited the summary of this revision. (Show Details)Aug 1 2018, 10:25 AM
khanchi97 updated this revision to Diff 9678.
pulkit requested changes to this revision.Aug 1 2018, 4:40 PM
pulkit added inline comments.
mercurial/commands.py
4537

please break this down into two if's. First one if all and confirm and the promptchoice in another one.

mercurial/configitems.py
193

You need to document this config option. You can have a look at where other options are documented and do something similar.

tests/test-resolve.t
498

this should be user quit rather than this abort.

This revision now requires changes to proceed.Aug 1 2018, 4:40 PM
khanchi97 updated this revision to Diff 9785.Aug 2 2018, 4:25 AM
pulkit added inline comments.Aug 2 2018, 6:12 PM
mercurial/commands.py
4540

This should be an abort with message 'user quit' instead of just a return.

khanchi97 updated this revision to Diff 9816.Aug 3 2018, 3:39 AM
pulkit accepted this revision.Aug 3 2018, 10:00 AM
This revision was automatically updated to reflect the committed changes.
martinvonz added inline comments.
mercurial/configitems.py
193

Is this name too generic? It's specifically for confirming re-resolve with --all. I can imagine having confirmation for marking files as resolved and I'm not sure we'd want to share the config option if we ever added such a feature. It might also make the name clearer if it was something like resolve.confirmreresolve or resolve.confirmredo or resolve.confirmoverwrite.

pulkit added inline comments.Aug 3 2018, 1:32 PM
mercurial/configitems.py
193

For now, yes the name is generic. My plan with @khanchi97 on this is to respect this option with other flags too. Like hg resolve -m will ask before marking as resolve if this config option is set.

Once we are done with other flags too, this won't sound generic.