protesilaos / denote

Simple notes for Emacs with an efficient file-naming scheme
https://protesilaos.com/emacs/denote
GNU General Public License v3.0
545 stars 55 forks source link

denote-rename-file does not remove the older file #402

Closed relict007 closed 3 months ago

relict007 commented 4 months ago

Hi Prot,

I am on the latest denote as of today (commit 8ad0b816ede166998fb73076b882b6526fa54f63). Sometimes when I use denote-rename-file (or any other rename function), I see that it creates a new file with updated name and keywords, however, it doesn't delete the older file. It does not happen everytime, and I have seen it to be happening only for binary files (pdf in my case).

I did a little debugging and problem seems to be happening because of the function denote-rename-file-and-buffer and suspect that it has something to do with commit d14a421036561cf6ae0505bbe1b80005290a690d. I don't see the problem if I revert this commit.

Could you please have a look.

protesilaos commented 4 months ago

From: relict007 @.***> Date: Thu, 1 Aug 2024 04:11:24 -0700

[... 7 lines elided]

Could you please have a look.

Sure! What version of Emacs are you on? We have not changed something to those renaming functions, so it may be a change in Emacs that is causing this.

-- Protesilaos Stavrou https://protesilaos.com

relict007 commented 4 months ago

This is my version:

GNU Emacs 29.3 (build 1, x86_64-pc-linux-gnu, cairo version 1.18.0) of 2024-05-26

protesilaos commented 4 months ago

I did a little debugging and problem seems to be happening because of the function denote-rename-file-and-buffer and suspect that it has something to do with commit d14a421. I don't see the problem if I revert this commit.

I am using this commit and everything seems to be working fine. Can you tell me more about the use-case here? Are you renaming files that already have a Denote file name, or not? Is this is the denote-directory or a silo?

relict007 commented 4 months ago

Are you renaming files that already have a Denote file name, or not?

Yes, its already a denote file. I am trying to rename it (and change the keywords).

Is this is the denote-directory or a silo?

This file is within denote-directory. I am not using any silos.

One more strange thing that I just tested is that problem doesn't occur when this file is in denote-directory/refile directory but occurs when its in denote-directory/files/documents directory. All these are just subdirs that I use to organize my files.

protesilaos commented 4 months ago

From: relict007 @.***> Date: Thu, 1 Aug 2024 05:07:01 -0700

[... 5 lines elided]

Is this is the denote-directory or a silo?

This file is within denote-directory. I am not using any silos.

One more strange thing that I just tested is that problem doesn't occur when this file is in denote-directory/refile directory but occurs when its in denote-directory/files/documents directory. All these are just subdirs that I use to organize my files.

I cannot reproduce this either. I am not sure what is happening here. Can you try to reproduce the issue by starting Emacs from the command line with 'emacs -Q'? Then set up Denote with the following:

(require 'package)

(package-initialize)

(require 'denote)

-- Protesilaos Stavrou https://protesilaos.com

relict007 commented 4 months ago

I ran emacs -Q and in the scratch buffer eval'd this. Copied some of the code from denote manual.

(require 'package)
(package-initialize)
(require 'denote "path-to-denote.el")
(setq denote-directory (expand-file-name "~/notes/denote"))
(setq denote-infer-keywords t)
(setq denote-sort-keywords t)
(setq denote-file-type nil) ; Org is the default, set others here
(setq denote-dired-directories
      (list denote-directory
            (thread-last denote-directory (expand-file-name "attachments"))
            (expand-file-name "~/Documents")))

I was able to rename a file in denote-directory, denote showed the file rename prompt and file was renamed correctly. However, same did not work in the directory denote-directory/documents/files. Denote asked the title and keywords but after saying y nothing happened (file did not get renamed at all).

I ran some more tests, and it seems to be happening to pdf file only (not getting renamed at all). org , html and zip got renamed properly.

This is very odd. Having a pdf filetype should not matter at all in this case. I will run some more tests.

relict007 commented 4 months ago

Just to clarify above, org, html and zip files are able to be renamed in any directory, however, pdf doesn't.

relict007 commented 3 months ago

I think I now know what the problem is. The rename commands fail for a file which is readonly. rename commands create another file (of 0 bytes) but fail to remove the previous file.

protesilaos commented 3 months ago

fail for a file which is readonly

Good find! I guess we can ask @jeanphilippegg about the following commit:

commit d14a421036561cf6ae0505bbe1b80005290a690d
Author: Jean-Philippe Gagné Guay <jeanphilippe150@gmail.com>
Date:   Thu May 16 20:14:20 2024 -0400

    Only rename the file if it exists in denote-rename-file-and-buffer

Do you remember what the issue was before adding the check for writable files?

jeanphilippegg commented 3 months ago

Sorry for the delay.

The function denote-rename-file-and-buffer has 2 independent purposes:

Prior to version 3.0.0, if you were in the process of creating a new note, you could not call denote-rename-file until you saved the current note (because the file did not exist yet and denote-rename-file-and-buffer would fail). This function needed to be modified to rename the file only if it exists. It was necessary for #279.

Of course, the -is-writable check on the old-file-name does not work with readonly files. We should remove this part of the check, but still validate that there is a file to rename.

We may also want to add some documentation to this function.

Let me know if you want me to do a pull request for this.

protesilaos commented 3 months ago

From: Jean-Philippe Gagné Guay @.***> Date: Fri, 16 Aug 2024 18:23:31 -0700

Sorry for the delay.

No problem! I hope you are doing well.

The function denote-rename-file-and-buffer has 2 independent purposes:

  • It renames an actual file on the system, if it exists.
  • It renames an Emacs buffer, if the file is open.

Prior to version 3.0.0, if you were in the process of creating a new note, you could not call denote-rename-file until you saved the current note (because the file did not exist yet and denote-rename-file-and-buffer would fail). This function needed to be modified to rename the file only if it exists. It was necessary for #279.

Ah yes, this is fine.

Of course, the -is-writable check on the old-file-name does not work with readonly files. We should remove this part of the check, but still validate that there is a file to rename.

We may also want to add some documentation to this function.

Let me know if you want me to do a pull request for this.

Please go ahead!

-- Protesilaos Stavrou https://protesilaos.com

jeanphilippegg commented 3 months ago

No problem! I hope you are doing well.

Yes, I am good! Hope you are doing well too!

I have made pull request #414 for this.

protesilaos commented 3 months ago

Thank you @jeanphilippegg! I just merged your PR. Closing now.