Enhancing denote-rewrite-front-matter to save and kill buffer when it's safe to do so

[mentalisttraceur]

Here's a huuuge (for me) UX enhancement that I monkey-patch in from my config:

(defun fixed-denote-rewrite-front-matter
        (denote-rewrite-front-matter path &rest arguments)
    (let* ((buffers (buffer-list))
           (buffer  (find-file-noselect path))
           (_ (refresh-modified-state buffer))
           (had-unsaved-changes (buffer-modified-p buffer)))
        (apply denote-rewrite-front-matter path arguments)
        (unless had-unsaved-changes
            (with-current-buffer buffer
                (basic-save-buffer))
            (unless (memq buffer buffers)
                (kill-buffer buffer)))))
(advice-add 'denote-rewrite-front-matter
    :around 'fixed-denote-rewrite-front-matter)

Big-picture overview:

1. If the buffer and file were the same before we rewrote the frontmatter, it's safe and nicer UX to automatically save it,

2. if the file was also not even open before we rewrote the frontmatter, it's safe and nicer UX to automatically close it after saving it.

Obviously "safe" here just means as safe as me manually doing that right after, and "nicer UX" here means that I always would manually do that right after.

Now the key piece here for correctness is refresh-modified-state. That's my own invention - it forces Emacs to re-check if buffer contents differ from the visited file. This, in my view, is essential. If the file changed on disk and buffer-modified-p is out of sync, automatically saving would be unacceptably prone to data loss. But once we can force a refresh of that state, we get better safety against data-losing race conditions than a manual check of the same (because the time it takes for a human to manually save the file after checking for buffer-vs-file differences is orders of magnitude longer than the time it takes for the code to both modify the frontmatter and save).

Anyway, my joy using Denote (and especially building other things with Denote as a building block) went up a lot ever since I implemented this wrapper. I've been using this for several months now with no issues, never looked back, don't want to ever go back. Hopefully it helps others too.

If there's interest in upstreaming this, I have some intent to eventually publish buffer-differs-from-visited-file-p and refresh-modified-state to ELPA, but I haven't yet signed GNU papers. In the meantime, the default license on code posted to StackOverflow/Exchange is permissive (MIT iirc) and I also offer all of that code under 0BSD, which is widely recognized as public-domain-equivalent.

If not, feel free to close it and it can just help people who come searching/asking for this feature.

[jeanphilippegg]

The latest versions of those functions have a :save-buffer optional parameter that allows saving the buffer after renaming. They are still not closed automatically though. There are many situations in which Emacs is not deleting buffers automatically (Dired, Magit, Org agenda, etc.). I do not know if there is a good reason for that.

[mentalisttraceur]

That's good, but I'd rarely want to simply unconditionally save the buffer, because

1. if I already have unsaved changes, they're unsaved for a reason, and
2. all I said above about safety if the file was changed outside of Emacs.

So this only reduces my wrapper like this:

 (defun fixed-denote-rewrite-front-matter
         (denote-rewrite-front-matter path &rest arguments)
     (let* ((buffers (buffer-list))
            (buffer  (find-file-noselect path))
            (_ (refresh-modified-state buffer))
            (had-unsaved-changes (buffer-modified-p buffer)))
-        (apply denote-rewrite-front-matter path arguments)
+        (apply denote-rewrite-front-matter path :save-buffer (not had-unsaved-changes) arguments)
         (unless had-unsaved-changes
-            (with-current-buffer buffer
-                (basic-save-buffer))
             (unless (memq buffer buffers)
                 (kill-buffer buffer)))))

[mentalisttraceur]

Re: buffers: I think there's several good reasons, most not applicable here (but it's not like I would be against a parameter to keep the buffers open unconditionally if someone wanted that):

1. Those things leave around buffers that you opened. They don't create and leabe a bunch of persistent buffers that you never even saw. My wrapper above only automatically closes buffers if they weren't even open when you ran the rename - in other words, situations where you never even see the buffer open, because you just did the rename on the file under point in Dired or whatever.

2. Navigation. You can quickly find buffers you already opened.

   • I suspect this is part of why Dired does this: so you don't have to just navigate by Dired links and new calls to dired.

   • This is why I advise the help functions in my config to make a new help buffer for each topic (when I run describe-function on denote, I get a help buffer called *Help (function: denote)* and the main benefit is that I can then very quickly go to or search for any help page I've visited.

3. Persistence. You might have state you don't want to lose in the buffer, so you bury by default rather than killing.

   • Dired might have marks/etc. (Of course if this was the sole reason, they could kill by default and bury if there's marks.)

   • Magit might have some partially finished procedure. Similarly, my own Git interface has my quitting keys bound to quit-window without killing because if I still have a git command running in it, I don't want to kill it or lose it.

4. (historical reason) Efficiency. Creating buffers is cheap, but "expensive" relative to just reusing the buffer you already have. (Doesn't really matter with modern machines unless you're recreating buffers in a hot loop or something, but something like Dired hails from the 1970s/1980s when needlessly allocating a few thousand bytes, running an ls, and parsing+coloring ls output, all while the old almost identical buffer was being garbage collected, could actually be noticably expensive).

5. Invariants. Complex stateful packages might benefit from the simplicity or guarantees of having the same buffer persist in the background.

6. Review. Sometimes you want to review what you did or what happened.

   • In my Git interface, old commands are left in the buffer scrollback. So if you staged a hunk at some point, if you haven't yet killed the git add -p buffer for that repo or file, you can go back and find that you staged it, and when you did so. If a push had an error hours ago and you want to show it to someone, if you haven't killed the buffer that day you can go back in the git push buffer and see it again.

There's probably other reasons, but I think that covers almost all of them. And I don't think any of that applies to buffers of files only opened automatically in the background to do the front matter part of the rename, if they're also automatically saved.

[mentalisttraceur]

Actually, you could just add a :kill-buffer parameter!

Then I can simplify my wrapper down to

(let* ((buffer (find-buffer-visiting path))
       (unsaved-changes (when buffer
                           (refresh-modified-state buffer)
                           (buffer-modified-p buffer))))
    (denote-whatever
        ...
        :save-buffer (not unsaved-changes)
        :kill-buffer (not buffer)))

1. Simpler for you, no coupling to my code to refresh buffer's modified state. I think I even like that orthogonality better.
2. Still an improvement on my end too.
3. Kinda sucks that other users don't benefit like I do though... but I guess they should've thought about that before deciding to be someone other than me maybe we could toss a note in the docs about how to put these two pieces together.