GNU bug report logs - #5436
23.1.91; Deleting directories causes unusable file layout in freedesktop trashcan

Previous Next

Full log

Message #20 received at 5436 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: David De La Harpe Golden <david <at> harpegolden.net>
Cc: cyd <at> stupidchicken.com, tassilo <at> member.fsf.org, 5436 <at> debbugs.gnu.org
Subject: Re: bug#5436: 23.1.91;
	Deleting directories causes unusable file layout in
	freedesktop	trashcan
Date: Sun, 24 Jan 2010 20:24:37 +0200

> Date: Sun, 24 Jan 2010 04:14:10 +0000
> From: David De La Harpe Golden <david <at> harpegolden.net>
> Cc: Tassilo Horn <tassilo <at> member.fsf.org>, 5436 <at> debbugs.gnu.org
> 
> Please find attached small* patch that should address this (and Bug#3353).

Thanks.

> -	   ;; We do not want to delete "." and "..".

This comment was not added to the new code.  I think it's useful, even
though the name directory-files-no-dot-files-regexp hints about the
intent.

> +      ;; move-file-to-trash moves directories, empty or
> +      ;; otherwise, into the trash as a unit. So if the directory is
> +      ;; non-empty, only call move-file-to-trash here if recursive is non-nil,
> +      ;; so that having delete-by-moving-trash non-nil doesn't cause "greedier"
> +      ;; (pseudo-)deletion behaviour.

Looks like this comment was not filled (via M-q).  Also, please always
leave two blanks after the period that ends a sentence.

> +      (if delete-by-moving-to-trash
> +          (if (and (not recursive)
> +                   (directory-files
> +                    directory 'full directory-files-no-dot-files-regexp))
> +              (error "Directory is not empty, not moving to trash.")
> +            (move-file-to-trash directory))

Why error out here?  Isn't it better to display a message and continue
(with other potential moves)?  I'm frequently annoyed by a tool's
tendency to give up early leaving me in the middle of a partly done
job.

> +          ;; do recursion if necessary if we're not moving to trash

Full sentences beginning with a capital letter, please.

> +deleting files outright. If FILENAME is a directory, it
                          ^^
One more space.

> +/* Lisp function for recursively copying directories. */
                                                        ^
Two spaces here.

> +    if (!NILP (Ffile_directory_p (file)))
> +      {
> +        newname = Fexpand_file_name (Ffile_name_nondirectory (Fdirectory_file_name (file)), newname);
> +      }
> +    else
> +      {
> +        newname = Fexpand_file_name (Ffile_name_nondirectory (file), newname);
> +      }

No need for braces if there's only one line in the clause.

Thanks again for working on this.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.