This is an archive of the discontinued Mercurial Phabricator instance.

purge: move extension into core mercurial
AbandonedPublic

Authored by valentin.gatienbaron on Nov 8 2020, 6:30 PM.

Details

Reviewers
marmoute
Group Reviewers
hg-reviewers
Summary

The motivation is simple: it's nicer to avoid gating basic
functionality.

To reduce the risk of people shooting themselves in the feet, `hg
purge` without further argument fails. Instead we add a flag -u that
means "delete unknown files", and the user must pass one of -u,
-i, or --all.

When the extension is loaded, -u is implied for compatibility, but
it can be passed explicitly, so one can call hg purge -u and have
that work regardless of whether the extension is loaded.

For review of the body of the purge command, use this instead of what
hg/phabricator will show (the block of code is modified, not just
moved):

diff -U 1000 <(hg cat -r .^ hgext/purge.py) <(cat mercurial/commands.py | grep -B 1 -A 1000 'purge|clean' | grep -B 1000 "ui.write") | colordiff | less -R

This is not exactly what was discussed at the sprint (people mentioned
an interactive prompt if some flag is not given), but this seems more
natural and as safe?

Separately, we could, in principle, prevent prefixes of hg pur and
hg cle from running purge, if people are still worried about users
typing the wrong things.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

The proposal during the spring was to ask by default if it is not loaded as extension. Or to make it more explicit:
(1) Check if there is anything to do. If no, just return success.
(2) Check if the extension was enabled. If yes, remove and exit.
(3) Check if the "--confirm" (or "--force"?) flag was given. If not, ask for confirmation to remove X files.
(4) Remove files.

It would be nice to not allow abbreviations of the command name. I'd even be fully OK with only enabled the "clean" alias for BC when the extension is loaded.

Should this extension be added to mercurial.extensions._builtin like color, progress and shelve? (I see pager isn't, so I'm not clear what this list does.)

The other thought I had was should extensions.purge=! in the config disable the command (due to the potential footgun)? I thought a similar config would disable the builtin color or pager, but some experimenting seems to show that isn't the case, so IDK where I got that idea. I don't feel strongly about this, and a followup would be fine if anyone else thinks it's a safety factor with merit.

I definitely like the idea of not being interactive, so that scripts can more easily purge.

@joerg.sonnenberger are you stating what was discussed at the sprint merely for comparison, or because you'd prefer for this change to be closer to it?

@mharbison72 the extension is not listed as builtin, because there is still a purge.py

I don't think just aborting when no option is given is a good UX. That feels even worse than making it interactive by default as discussed during the sprint.

Failing is what the vast majority of commands do when not given enough arguments:

$ hg clone
hg clone: invalid arguments
...
$ hg backout
abort: please specify a revision to backout
$ hg tag
hg tag: invalid arguments
...
$ hg resolve
abort: no action specified
...
$ hg revert
abort: no files or directories specified
...

prune is probably most similar to hg resolve or hg revert, in that they could interpret no arguments in some way, but chose not to, as it can be error prone.
If one runs hg clean without further argument, I think "clean what? (ignored files, unclean files, both)" is a better response than "are you sure you want to delete these unclean files?".

I'm -0 on this, but open to persuasion: I think that an interactive mode that lists files and then says "are you sure" makes sense. eg:

$ hg purge --all
foo.sh
build/
junk
delete listed files/directories? [y/N]

or

$ hg purge --all
foo
bar
baz
quxx
[and 200 more files and directories]
proceed? [y/N]

That is, show a summary of the files/directories (if we're deleting an entire dir, just list that, not every file in it, ideally) to the user, brief enough it's likely to fit on their terminal, and then ask. Otherwise I really fear the "Mercurial ate my homework" complaints are inevitable.

pulkit added a subscriber: pulkit.Nov 19 2020, 8:23 AM

I'm -0 on this, but open to persuasion: I think that an interactive mode that lists files and then says "are you sure" makes sense. eg:

I agree that interactive mode makes sense here. Since there is no way back from purging, we need to be more careful.

$ hg purge --all
foo.sh
build/
junk
delete listed files/directories? [y/N]

or

$ hg purge --all
foo
bar
baz
quxx
[and 200 more files and directories]
proceed? [y/N]

To add we can add a --confirm flag which defaults to True and one can pass --no-confirm to not have interactive option.
(I missed the sprint discussion and hence kindly ignore me if I am saying something that's already discussed)

That is, show a summary of the files/directories (if we're deleting an entire dir, just list that, not every file in it, ideally) to the user, brief enough it's likely to fit on their terminal, and then ask. Otherwise I really fear the "Mercurial ate my homework" complaints are inevitable.

marmoute requested changes to this revision.Jan 8 2021, 2:37 PM
marmoute added a subscriber: marmoute.

If I remember the sprint discussion right, the idea was

  • add to core with a --confirm option (default to False),
  • have the extensions change --confirm default to True
This revision now requires changes to proceed.Jan 8 2021, 2:37 PM

Failing is what the vast majority of commands do when not given enough arguments:

$ hg clone
hg clone: invalid arguments
...
$ hg backout
abort: please specify a revision to backout
$ hg tag
hg tag: invalid arguments
...
$ hg resolve
abort: no action specified
...
$ hg revert
abort: no files or directories specified
...

prune is probably most similar to hg resolve or hg revert, in that they could interpret no arguments in some way, but chose not to, as it can be error prone.
If one runs hg clean without further argument, I think "clean what? (ignored files, unclean files, both)" is a better response than "are you sure you want to delete these unclean files?".

Having some flag to control what you delete would be great (probably --unknown, --ignored and --all), but since they are deleted for good without any backup (unlike resolve and revert), I still think having some explicite validation would be good. That validation could be something like:

$ hg clean --all
permanently delete 3 unknown file and 250 ignored files ? [yN]
(use `hg status --unknown --ignored` to list them)

Having some hint about what the command is about to do is probably enough. My main source of un-recoverable error with hg purge is when I accidentally remove an unexpected "unknown" file while trying to client the "ignored" one.

I gave a shot at a --confirm option. An alternative series to this one can be found in D9818 D9819 and D98120