[PATCH] Add a new option '-k --keepdirs' to the 'remove' command, which, if specified on command line, does not delete empty directories recursively.

Greg Ward greg-hg at gerg.ca
Fri Aug 28 08:04:15 CDT 2009


> # 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...')),
>      "rename|mv":

The usual convention for multi-word options is to use a hyphen, so
that should be 'keep-dirs'.  E.g. 'log --follow-first', 'commit
--close-branch'.

Also, it's debatable whether this option deserves a one-letter short
form, since it's a fairly arcane use case.

> @@ -445,14 +445,15 @@
>          os.unlink(temp)
>          os.rename(src, dst)
>
> -def unlink(f):
> +def unlink(f, keepdirs=None):
>      """unlink and remove the directory if it is empty"""
>      os.unlink(f)
>      # 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!

Greg



More information about the Mercurial-devel mailing list