Eli Zaretskii wrote: >> Date: Sun, 24 Jan 2010 04:14:10 +0000 >> From: David De La Harpe Golden >> Cc: Tassilo Horn , 5436@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. The comment was not appropriate - Beware the same test is for a different reason in the new bit of code. Anyway, I've tried to rephrase (and, yes, fill) the new comment explaining that better in the attached. >> + (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? Again, specifically because the non- delete-by-moving-to-trash case does - delete-directory only deletes non-empty directories if you ask for a recursive delete, otherwise it will error out if the directory is non-empty. That may not be completely obvious from the lisp code since it happens when delete-directory-internal calls rename() in the non-trash case. I wanted to avoid inconsistent behaviour between the two cases, which is what the comment was documenting in fact, though it obviously wasn't clear enough the first time around. I for one _really_ don't like the idea of delete-directory acting differently in the trash and non-trash cases, although i suppose it is slightly "safer" in the trash case. Still, I would strongly recommend consistency between the two cases. Going the other way to achieve consistency, "fixing" the non-delete-by-moving-to-trash case to successfully recursively delete even when a recursive delete was not requested at all seems ...not a good idea... > No need for braces if there's only one line in the clause. Yeah, something in C that I tend to dislike. Nonetheless, you're of course correct for emacs source code conventions, fixed.