Patchwork purge: add options for deleting only files or only directories

login
register
mail settings
Submitter Ben Kehoe
Date Nov. 8, 2013, 1:29 a.m.
Message ID <8d80f5d4459447b7b855.1383874148@surgical5>
Download mbox | patch
Permalink /patch/2868/
State Superseded
Commit 8127b9e798b1ef79fe22cca671261cacbb36e72d
Headers show

Comments

Ben Kehoe - Nov. 8, 2013, 1:29 a.m.
# HG changeset patch
# User Ben Kehoe <benk@berkeley.edu>
# Date 1383873854 28800
#      Thu Nov 07 17:24:14 2013 -0800
# Node ID 8d80f5d4459447b7b85571dba6274e6ed8ab2c36
# Parent  c38c3fdc8b9317ba09e03ab09364c3800da7c50c
purge: add options for deleting only files or only directories

Added --files and --dirs options to purge, since it is often useful to leave
directory structure intact when deleting untracked files.
Either or both options can be given. If neither are given, it's the same as
both (i.e., existing behavior).
Augie Fackler - Nov. 16, 2013, 5:44 p.m.
On Thu, Nov 07, 2013 at 05:29:08PM -0800, Ben Kehoe wrote:
> # HG changeset patch
> # User Ben Kehoe <benk@berkeley.edu>
> # Date 1383873854 28800
> #      Thu Nov 07 17:24:14 2013 -0800
> # Node ID 8d80f5d4459447b7b85571dba6274e6ed8ab2c36
> # Parent  c38c3fdc8b9317ba09e03ab09364c3800da7c50c
> purge: add options for deleting only files or only directories

I think I like this. Anyone else have an opinion?

>
> Added --files and --dirs options to purge, since it is often useful to leave
> directory structure intact when deleting untracked files.
> Either or both options can be given. If neither are given, it's the same as
> both (i.e., existing behavior).
>
> diff -r c38c3fdc8b93 -r 8d80f5d44594 hgext/purge.py
> --- a/hgext/purge.py	Thu Nov 07 15:24:23 2013 -0600
> +++ b/hgext/purge.py	Thu Nov 07 17:24:14 2013 -0800
> @@ -35,6 +35,8 @@
>  @command('purge|clean',
>      [('a', 'abort-on-err', None, _('abort if an error occurs')),
>      ('',  'all', None, _('purge ignored files too')),
> +    ('',  'dirs', None, _('purge directories')),
> +    ('',  'files', None, _('purge files')),
>      ('p', 'print', None, _('print filenames instead of deleting them')),
>      ('0', 'print0', None, _('end filenames with NUL, for use with xargs'
>                              ' (implies -p/--print)')),
> @@ -46,7 +48,7 @@
>      Delete files not known to Mercurial. This is useful to test local
>      and uncommitted changes in an otherwise-clean source tree.
>
> -    This means that purge will delete:
> +    This means that purge will delete the following by default:
>
>      - Unknown files: files marked with "?" by :hg:`status`
>      - Empty directories: in fact Mercurial ignores directories unless
> @@ -58,6 +60,10 @@
>      - Ignored files (unless --all is specified)
>      - New files added to the repository (with :hg:`add`)
>
> +    The --files and --dirs options can be use to direct purge to delete
> +    only files, only directories, or both. If neither option is given,
> +    both will be deleted.
> +
>      If directories are given on the command line, only files in these
>      directories are considered.
>
> @@ -71,6 +77,11 @@
>      if opts['print0']:
>          eol = '\0'
>          act = False # --print0 implies --print
> +    removefiles = opts['files']
> +    removedirs = opts['dirs']
> +    if not removefiles and not removedirs:
> +        removefiles = True
> +        removedirs = True
>
>      def remove(remove_func, name):
>          if act:
> @@ -100,11 +111,13 @@
>      match.explicitdir = match.traversedir = directories.append
>      status = repo.status(match=match, ignored=opts['all'], unknown=True)
>
> -    for f in sorted(status[4] + status[5]):
> -        ui.note(_('removing file %s\n') % f)
> -        remove(removefile, f)
> +    if removefiles:
> +        for f in sorted(status[4] + status[5]):
> +            ui.note(_('removing file %s\n') % f)
> +            remove(removefile, f)
>
> -    for f in sorted(directories, reverse=True):
> -        if match(f) and not os.listdir(repo.wjoin(f)):
> -            ui.note(_('removing directory %s\n') % f)
> -            remove(os.rmdir, f)
> +    if removedirs:
> +        for f in sorted(directories, reverse=True):
> +            if match(f) and not os.listdir(repo.wjoin(f)):
> +                ui.note(_('removing directory %s\n') % f)
> +                remove(os.rmdir, f)
> diff -r c38c3fdc8b93 -r 8d80f5d44594 tests/test-purge.t
> --- a/tests/test-purge.t	Thu Nov 07 15:24:23 2013 -0600
> +++ b/tests/test-purge.t	Thu Nov 07 17:24:14 2013 -0800
> @@ -215,4 +215,50 @@
>    $ hg purge -p -X .svn -X '*/.svn'
>    $ hg purge -p -X re:.*.svn
>
> +  $ rm -R .svn directory r1
> +
> +only remove files
> +
> +  $ mkdir -p empty_dir dir
> +  $ touch untracked_file dir/untracked_file
> +  $ hg purge -p --files
> +  dir/untracked_file
> +  untracked_file
> +  $ hg purge -v --files
> +  removing file dir/untracked_file
> +  removing file untracked_file
> +  $ ls
> +  dir
> +  empty_dir
> +  $ ls dir
> +
> +only remove dirs
> +
> +  $ mkdir -p empty_dir dir
> +  $ touch untracked_file dir/untracked_file
> +  $ hg purge -p --dirs
> +  empty_dir
> +  $ hg purge -v --dirs
> +  removing directory empty_dir
> +  $ ls
> +  dir
> +  untracked_file
> +  $ ls dir
> +  untracked_file
> +
> +remove both files and dirs
> +
> +  $ mkdir -p empty_dir dir
> +  $ touch untracked_file dir/untracked_file
> +  $ hg purge -p --files --dirs
> +  dir/untracked_file
> +  untracked_file
> +  empty_dir
> +  $ hg purge -v --files --dirs
> +  removing file dir/untracked_file
> +  removing file untracked_file
> +  removing directory empty_dir
> +  removing directory dir
> +  $ ls
> +
>    $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Mads Kiilerich - Nov. 16, 2013, 5:55 p.m.
On 11/16/2013 12:44 PM, Augie Fackler wrote:
> On Thu, Nov 07, 2013 at 05:29:08PM -0800, Ben Kehoe wrote:
>> # HG changeset patch
>> # User Ben Kehoe <benk@berkeley.edu>
>> # Date 1383873854 28800
>> #      Thu Nov 07 17:24:14 2013 -0800
>> # Node ID 8d80f5d4459447b7b85571dba6274e6ed8ab2c36
>> # Parent  c38c3fdc8b9317ba09e03ab09364c3800da7c50c
>> purge: add options for deleting only files or only directories
> I think I like this. Anyone else have an opinion?

What are the good use cases?

It seems to me like the (admittedly very small) cost it has is bigger 
than the value it adds.

/Mads
Matt Mackall - Nov. 16, 2013, 7:51 p.m.
On Sat, 2013-11-16 at 12:55 -0500, Mads Kiilerich wrote:
> On 11/16/2013 12:44 PM, Augie Fackler wrote:
> > On Thu, Nov 07, 2013 at 05:29:08PM -0800, Ben Kehoe wrote:
> >> # HG changeset patch
> >> # User Ben Kehoe <benk@berkeley.edu>
> >> # Date 1383873854 28800
> >> #      Thu Nov 07 17:24:14 2013 -0800
> >> # Node ID 8d80f5d4459447b7b85571dba6274e6ed8ab2c36
> >> # Parent  c38c3fdc8b9317ba09e03ab09364c3800da7c50c
> >> purge: add options for deleting only files or only directories
> > I think I like this. Anyone else have an opinion?
> 
> What are the good use cases?
> 
> It seems to me like the (admittedly very small) cost it has is bigger 
> than the value it adds.

Perhaps we can do this with filesets.
Ben Kehoe - Nov. 16, 2013, 8:15 p.m.
On Sat, Nov 16, 2013 at 11:51 AM, Matt Mackall <mpm@selenic.com> wrote:

> On Sat, 2013-11-16 at 12:55 -0500, Mads Kiilerich wrote:
> > On 11/16/2013 12:44 PM, Augie Fackler wrote:
> > > On Thu, Nov 07, 2013 at 05:29:08PM -0800, Ben Kehoe wrote:
> > >> # HG changeset patch
> > >> # User Ben Kehoe <benk@berkeley.edu>
> > >> # Date 1383873854 28800
> > >> #      Thu Nov 07 17:24:14 2013 -0800
> > >> # Node ID 8d80f5d4459447b7b85571dba6274e6ed8ab2c36
> > >> # Parent  c38c3fdc8b9317ba09e03ab09364c3800da7c50c
> > >> purge: add options for deleting only files or only directories
> > > I think I like this. Anyone else have an opinion?
> >
> > What are the good use cases?
> >
> > It seems to me like the (admittedly very small) cost it has is bigger
> > than the value it adds.
>
> Perhaps we can do this with filesets.
>
> The primary motivation was that when mercurial is being used to version
the state of another program, that program may only do directory creation
at an initial point, and then afterwards assume the directory structure is
intact (poor design, but what can you do?), although some directories may
be empty (e.g., bin before a build is run). Restoring a previous state
would then be:
hg revert -aC && hg purge --files
The code for purge makes a clear distinction between files and directories,
which is why it seemed natural to add those simple options.
Ben Kehoe - Nov. 16, 2013, 9:03 p.m.
On Sat, Nov 16, 2013 at 12:54 PM, Kevin Bullock <
kbullock+mercurial@ringworld.org> wrote:

> On 16 Nov 2013, at 3:15 PM, Ben Kehoe <benk@berkeley.edu> wrote:
>
> > On Sat, Nov 16, 2013 at 11:51 AM, Matt Mackall <mpm@selenic.com> wrote:
> >
> >> Perhaps we can do this with filesets.
> >
> > The primary motivation was that when mercurial is being used to version
> the state of another program, that program may only do directory creation
> at an initial point, and then afterwards assume the directory structure is
> intact (poor design, but what can you do?), although some directories may
> be empty (e.g., bin before a build is run). Restoring a previous state
> would then be:
> > hg revert -aC && hg purge --files
> > The code for purge makes a clear distinction between files and
> directories, which is why it seemed natural to add those simple options.
>
> I agree with Matt, I think for the user-facing portion of this it doesn't
> make much sense to add options when we have a mechanism for specifying
> arbitrary groups of files. I'm not sure how useful it will be outside purge
> to specify just regular files or just directories, but I _can_ see a use
> case for e.g. specifying all executable files.
>

Maybe I don't understand filesets well enough, but what are the filesets
that would indicate "all directories" and "all files (but not directories)"?
Ben Kehoe - Nov. 18, 2013, 9:44 p.m.
On Sat, Nov 16, 2013 at 5:09 PM, Kevin Bullock <
kbullock+mercurial@ringworld.org> wrote:

> On 16 Nov 2013, at 4:03 PM, Ben Kehoe <benk@berkeley.edu> wrote:
>
> > On Sat, Nov 16, 2013 at 12:54 PM, Kevin Bullock <
> kbullock+mercurial@ringworld.org> wrote:
> >
> >> I agree with Matt, I think for the user-facing portion of this it
> doesn't make much sense to add options when we have a mechanism for
> specifying arbitrary groups of files. I'm not sure how useful it will be
> outside purge to specify just regular files or just directories, but I
> _can_ see a use case for e.g. specifying all executable files.
> >>
> > Maybe I don't understand filesets well enough, but what are the filesets
> that would indicate "all directories" and "all files (but not directories)"?
>
> I think we're suggesting you implement them. :)
>
> Ah, I see. I don't mean to be petulant, but it seems to me like filesets
may not be the right solution. The semantics of the existing positional
arguments to purge expect a directory to be given and take this to mean
"all files and subdirectories within this directory", so to be consistent,
giving a fileset that only matched directories would still not produce the
effect of "delete only empty directories, not untracked files".
As a user, when I first encountered the purge extension, it was apparent
that it did two separate (though related) things: delete untracked files,
and delete empty directories. It seems to me like simply allowing these two
functions to be performed individually should be more appealing than adding
new semantics to filesets that are only relevant for an extension.
I could of course be totally missing the point here, in which case, please
enlighten me :-)
Pierre-Yves David - April 14, 2014, 4:05 a.m.
This discussion seems to have never been concluded?

Shall we consider the feature rejected?

On 11/18/2013 04:44 PM, Ben Kehoe wrote:
> On Sat, Nov 16, 2013 at 5:09 PM, Kevin Bullock
> <kbullock+mercurial@ringworld.org
> <mailto:kbullock+mercurial@ringworld.org>> wrote:
>
>     On 16 Nov 2013, at 4:03 PM, Ben Kehoe <benk@berkeley.edu
>     <mailto:benk@berkeley.edu>> wrote:
>
>      > On Sat, Nov 16, 2013 at 12:54 PM, Kevin Bullock
>     <kbullock+mercurial@ringworld.org
>     <mailto:kbullock%2Bmercurial@ringworld.org>> wrote:
>      >
>      >> I agree with Matt, I think for the user-facing portion of this
>     it doesn't make much sense to add options when we have a mechanism
>     for specifying arbitrary groups of files. I'm not sure how useful it
>     will be outside purge to specify just regular files or just
>     directories, but I _can_ see a use case for e.g. specifying all
>     executable files.
>      >>
>      > Maybe I don't understand filesets well enough, but what are the
>     filesets that would indicate "all directories" and "all files (but
>     not directories)"?
>
>     I think we're suggesting you implement them. :)
>
> Ah, I see. I don't mean to be petulant, but it seems to me like filesets
> may not be the right solution. The semantics of the existing positional
> arguments to purge expect a directory to be given and take this to mean
> "all files and subdirectories within this directory", so to be
> consistent, giving a fileset that only matched directories would still
> not produce the effect of "delete only empty directories, not untracked
> files".
> As a user, when I first encountered the purge extension, it was apparent
> that it did two separate (though related) things: delete untracked
> files, and delete empty directories. It seems to me like simply allowing
> these two functions to be performed individually should be more
> appealing than adding new semantics to filesets that are only relevant
> for an extension.
> I could of course be totally missing the point here, in which case,
> please enlighten me :-)
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Sean Farley - April 14, 2014, 4:14 a.m.
Pierre-Yves David <pierre-yves.david@ens-lyon.org> writes:

> This discussion seems to have never been concluded?

Good catch.

> Shall we consider the feature rejected?
>
> On 11/18/2013 04:44 PM, Ben Kehoe wrote:
>> On Sat, Nov 16, 2013 at 5:09 PM, Kevin Bullock
>> <kbullock+mercurial@ringworld.org
>> <mailto:kbullock+mercurial@ringworld.org>> wrote:
>>
>>     On 16 Nov 2013, at 4:03 PM, Ben Kehoe <benk@berkeley.edu
>>     <mailto:benk@berkeley.edu>> wrote:
>>
>>      > On Sat, Nov 16, 2013 at 12:54 PM, Kevin Bullock
>>     <kbullock+mercurial@ringworld.org
>>     <mailto:kbullock%2Bmercurial@ringworld.org>> wrote:
>>      >
>>      >> I agree with Matt, I think for the user-facing portion of this
>>     it doesn't make much sense to add options when we have a mechanism
>>     for specifying arbitrary groups of files. I'm not sure how useful it
>>     will be outside purge to specify just regular files or just
>>     directories, but I _can_ see a use case for e.g. specifying all
>>     executable files.
>>      >>
>>      > Maybe I don't understand filesets well enough, but what are the
>>     filesets that would indicate "all directories" and "all files (but
>>     not directories)"?
>>
>>     I think we're suggesting you implement them. :)
>>
>> Ah, I see. I don't mean to be petulant, but it seems to me like filesets
>> may not be the right solution. The semantics of the existing positional
>> arguments to purge expect a directory to be given and take this to mean
>> "all files and subdirectories within this directory", so to be
>> consistent, giving a fileset that only matched directories would still
>> not produce the effect of "delete only empty directories, not untracked
>> files".
>> As a user, when I first encountered the purge extension, it was apparent
>> that it did two separate (though related) things: delete untracked
>> files, and delete empty directories. It seems to me like simply allowing
>> these two functions to be performed individually should be more
>> appealing than adding new semantics to filesets that are only relevant
>> for an extension.
>> I could of course be totally missing the point here, in which case,
>> please enlighten me :-)

I have to agree with Ben here.

Patch

diff -r c38c3fdc8b93 -r 8d80f5d44594 hgext/purge.py
--- a/hgext/purge.py	Thu Nov 07 15:24:23 2013 -0600
+++ b/hgext/purge.py	Thu Nov 07 17:24:14 2013 -0800
@@ -35,6 +35,8 @@ 
 @command('purge|clean',
     [('a', 'abort-on-err', None, _('abort if an error occurs')),
     ('',  'all', None, _('purge ignored files too')),
+    ('',  'dirs', None, _('purge directories')),
+    ('',  'files', None, _('purge files')),
     ('p', 'print', None, _('print filenames instead of deleting them')),
     ('0', 'print0', None, _('end filenames with NUL, for use with xargs'
                             ' (implies -p/--print)')),
@@ -46,7 +48,7 @@ 
     Delete files not known to Mercurial. This is useful to test local
     and uncommitted changes in an otherwise-clean source tree.
 
-    This means that purge will delete:
+    This means that purge will delete the following by default:
 
     - Unknown files: files marked with "?" by :hg:`status`
     - Empty directories: in fact Mercurial ignores directories unless
@@ -58,6 +60,10 @@ 
     - Ignored files (unless --all is specified)
     - New files added to the repository (with :hg:`add`)
 
+    The --files and --dirs options can be use to direct purge to delete
+    only files, only directories, or both. If neither option is given,
+    both will be deleted.
+
     If directories are given on the command line, only files in these
     directories are considered.
 
@@ -71,6 +77,11 @@ 
     if opts['print0']:
         eol = '\0'
         act = False # --print0 implies --print
+    removefiles = opts['files']
+    removedirs = opts['dirs']
+    if not removefiles and not removedirs:
+        removefiles = True
+        removedirs = True
 
     def remove(remove_func, name):
         if act:
@@ -100,11 +111,13 @@ 
     match.explicitdir = match.traversedir = directories.append
     status = repo.status(match=match, ignored=opts['all'], unknown=True)
 
-    for f in sorted(status[4] + status[5]):
-        ui.note(_('removing file %s\n') % f)
-        remove(removefile, f)
+    if removefiles:
+        for f in sorted(status[4] + status[5]):
+            ui.note(_('removing file %s\n') % f)
+            remove(removefile, f)
 
-    for f in sorted(directories, reverse=True):
-        if match(f) and not os.listdir(repo.wjoin(f)):
-            ui.note(_('removing directory %s\n') % f)
-            remove(os.rmdir, f)
+    if removedirs:
+        for f in sorted(directories, reverse=True):
+            if match(f) and not os.listdir(repo.wjoin(f)):
+                ui.note(_('removing directory %s\n') % f)
+                remove(os.rmdir, f)
diff -r c38c3fdc8b93 -r 8d80f5d44594 tests/test-purge.t
--- a/tests/test-purge.t	Thu Nov 07 15:24:23 2013 -0600
+++ b/tests/test-purge.t	Thu Nov 07 17:24:14 2013 -0800
@@ -215,4 +215,50 @@ 
   $ hg purge -p -X .svn -X '*/.svn'
   $ hg purge -p -X re:.*.svn
 
+  $ rm -R .svn directory r1
+
+only remove files
+
+  $ mkdir -p empty_dir dir
+  $ touch untracked_file dir/untracked_file
+  $ hg purge -p --files
+  dir/untracked_file
+  untracked_file
+  $ hg purge -v --files
+  removing file dir/untracked_file
+  removing file untracked_file
+  $ ls
+  dir
+  empty_dir
+  $ ls dir
+
+only remove dirs
+
+  $ mkdir -p empty_dir dir
+  $ touch untracked_file dir/untracked_file
+  $ hg purge -p --dirs
+  empty_dir
+  $ hg purge -v --dirs
+  removing directory empty_dir
+  $ ls
+  dir
+  untracked_file
+  $ ls dir
+  untracked_file
+
+remove both files and dirs
+
+  $ mkdir -p empty_dir dir
+  $ touch untracked_file dir/untracked_file
+  $ hg purge -p --files --dirs
+  dir/untracked_file
+  untracked_file
+  empty_dir
+  $ hg purge -v --files --dirs
+  removing file dir/untracked_file
+  removing file untracked_file
+  removing directory empty_dir
+  removing directory dir
+  $ ls
+
   $ cd ..