[PATCH] Add a new option '-k --keepdirs' to the 'remove' command, which, if specified on command line, does not delete empty directories recursively.
peter.arrenbrecht at gmail.com
Fri Aug 28 08:27:08 CDT 2009
On Fri, Aug 28, 2009 at 3:04 PM, Greg Ward<greg-hg at gerg.ca> wrote:
>> # HG changeset patch
>> # User Jonny Dee <jonny.dee at gmx.net>
>> # Date 1251414081 -7200
>> # Node ID be9b3b933595ba0c88f01cdb9f2ccb3cc32b690d
>> # Parent 7345fa5e572e029362bc6f2f96af452607ebb1ff
>> Add a new option '-k --keepdirs' to the 'remove' command, which, if specified on command line, does not delete empty directories recursively.
> Please aim for a concise one-sentence description that fits in ~70
> chars with context as a prefix. More detail, if needed, on subsequent
> lines. E.g.:
> remove: add -k/--keep-dirs option to disable removing empty directories.
> Writing checkin messages is an art; writing one-line checkin messages
> for Mercurial is a particularly demanding art. They are picky around
> here. ;-)
>> @@ -3456,6 +3457,8 @@
>> [('A', 'after', None, _('record delete for missing files')),
>> ('f', 'force', None,
>> _('remove (and delete) file even if added or modified')),
>> + ('k', 'keepdirs', None,
>> + _('do not recursively delete empty directories')),
>> ] + walkopts,
>> _('[OPTION]... FILE...')),
> The usual convention for multi-word options is to use a hyphen, so
> that should be 'keep-dirs'. E.g. 'log --follow-first', 'commit
> Also, it's debatable whether this option deserves a one-letter short
> form, since it's a fairly arcane use case.
I'd drop the short form, too.
>> @@ -445,14 +445,15 @@
>> os.rename(src, dst)
>> -def unlink(f):
>> +def unlink(f, keepdirs=None):
>> """unlink and remove the directory if it is empty"""
>> # try removing directories that might now be empty
>> - try:
>> - os.removedirs(os.path.dirname(f))
>> - except OSError:
>> - pass
>> + if not keepdirs:
>> + try:
>> + os.removedirs(os.path.dirname(f))
>> + except OSError:
>> + pass
> Please update the docstring to describe 'keepdirs'.
> Four-space indentation, please.
> Other than those details, it's looking pretty good!
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
More information about the Mercurial-devel