Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhancing denote-rename-file to handle not-yet-saved visiting buffer #279

Closed
mentalisttraceur opened this issue Mar 8, 2024 · 5 comments

Comments

@mentalisttraceur
Copy link

mentalisttraceur commented Mar 8, 2024

I use denote-rename-file a lot for changing information that shows up in a Denoted file name.

Sometimes, I'll create a note, do some writing, and only then I'll think of some change to the title or the tags. But at that moment I often haven't saved the note yet and don't even know if I'm going to keep it - I don't want to save it just to change the relevant information.

So I think it would be a nice addition if denote-rename-file would work on files that don't exist, but for which a visiting buffer exists, doing everything except writing the file out to disk.


Architecturally, to me it makes a whole lot of sense, especially when using/considering Denote's public functions as building blocks to build even more good things on top of Denote's goodness, that

  1. there might reasonably be other possible use cases in things people build on top of Denote, for manipulating information that shows up in a Denoted file name before the first save of that file (which may or may not be a note), and
  2. having a single entry point to manipulate parts of a Denoted file name is very convenient. I take the existence of denote-rename-file, and the upcoming/mentioned change of making that function respect the prompts variable to control what it prompts for when used interactively, as agreement. (Consider the combinatoric explosion otherwise: right now I only have to learn+understand and wrap+advise one function - denote-rename-file - to get all the behavior that I want.)

My first solution to this was to have the following code in my denote-rename-file wrapper to make this happen. It was crude, but it was quick, simple, didn't require me to know or couple any implementation details, and it works "well enough" - it temporarily creates the file if it doesn't exist before running denote-rename-file and then deletes it right after:

            (if (file-exists-p path)
                (denote-rename-file path title keywords signature)
                (with-current-buffer (find-file-noselect path)
                    (write-file-no-visit path))
                (let ((new-path (denote-rename-file path title keywords signature)))
                    (delete-file new-path)
                    (refresh-modified-state)
                    new-path))))

But obviously, it would be cleaner to just operate on the visited buffer without touching the file system.

Currently I use this more invasive monkey-patching to achieve that:

    (defvar fixed-denote-rename-file--missing nil)

    (defun hack-rename-file (rename-file &rest arguments)
        (condition-case error
            (apply rename-file arguments)
            (file-missing
                (setq fixed-denote-rename-file--missing t))))

    (defun hack-denote--file-regular-writable-p
            (denote--file-regular-writable-p file)
        (if fixed-denote-rename-file--missing
            (find-buffer-visiting file)
            (funcall denote--file-regular-writable-p file)))

    (defun fixed-denote-rename-file (denote-rename-file &rest arguments)
        (with-advice ('rename-file :around 'hack-rename-file
                      'denote--file-regular-writable-p
                          :around 'hack-denote--file-regular-writable-p)
            (let ((fixed-denote-rename-file--missing nil))
                (apply denote-rename-file arguments))))

    (advice-add 'denote-rename-file :around 'fixed-denote-rename-file)

I suspect in-package support would be cleaner, simpler code that's easier to follow and maintain. If there's agreement that it's worth adding in-package, I can draft a full patch/PR.

@jeanphilippegg
Copy link
Contributor

I am surprised this does not already work, but I tested it by calling denote and then attempting denote-rename-file on it and it does not allow me to rename the note. It must be saved first. I think you are right and we should allow that.

I suspect in-package support would be cleaner, simpler code that's easier to follow and maintain. If there's agreement that it's worth adding in-package, I can draft a full patch/PR.

If you have not assigned copyright to the FSF, it would probably be best to continue opening issues for others to fix.

@mentalisttraceur
Copy link
Author

mentalisttraceur commented Mar 9, 2024

If you have not assigned copyright to the FSF, it would probably be best to continue opening issues for others to fix.

Cool. I'll hold off until that's done. This may finally get me to stop procrastinating on it.

@jeanphilippegg
Copy link
Contributor

I think this is fixed in the latest code, if you want to test it. Create a new note, call denote-rename-file and the new unsaved note is renamed (and still unsaved).

(Just make sure to revert the last commit, because it breaks the creation command. I'll fix it.)

@mentalisttraceur
Copy link
Author

Just got around to looking at this: works great, one more hacky monkey-patch removed from my config! Thanks @jeanphilippegg & @protesilaos !

@jeanphilippegg
Copy link
Contributor

You're welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants