protesilaos / denote

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

Streamline the renaming commands #333

Closed jeanphilippegg closed 4 months ago

jeanphilippegg commented 5 months ago

Even though this is a big pull request, it does not contain much user-visible changes. This is mostly all internal refactoring.

This pull request is the big one that streamlines all renaming commands. The idea is to build all renaming commands (thus removing code duplication) on top of 2 building blocks : denote--rename-get-file-info-from-prompts-or-existing and denote--rename-file, similar in principle to denote-rename-file-subr that we had at some point.

These 2 internal functions can be combined in various ways to get all the renaming commands, as you can see in this pull request. All commands behave exactly as before.

This is the user-visible changes:

jeanphilippegg commented 5 months ago

I have tested them with various combinations of denote-prompts, denote-rename-confirmations and denote-save-buffers. They work as expected!

jeanphilippegg commented 5 months ago

I just pushed one small adjustment. It is ready to be merged.

jeanphilippegg commented 5 months ago

I have added one convenience renaming command (denote-rename-file-title) as it may be useful.

This is a summary of the changes:

The rest is refactoring. Let me know if you want me to update the manual as well.

protesilaos commented 4 months ago

Hello @jeanphilippegg! I have not had the chance to review this. Will do it later today. Thank you!

jeanphilippegg commented 4 months ago

No worries! Take your time.

protesilaos commented 4 months ago

Merged. Thank you!

protesilaos commented 4 months ago

Note that the denote-rename-file-signature prompts to modify the front matter. This is a good time for us to think what we should do with signatures and front matter. Do we want to include them there? It has to be done in a backward-compatible way, which makes it trickier.

If we keep the status quo, then we need to make sure that no prompt for a change to the front matter happens when we just rename for a signature.

protesilaos commented 4 months ago

About this:

      ;; TODO: Offer to regenerate link descriptions in other files on
      ;; rename. Maybe this should be a distinct command.
      (when denote--used-ids
        (puthash id t denote--used-ids))

If we make it part of the renaming process, let us put it behind an opt-in user option. Otherwise it adds more friction. For example, I frequently rename files that do not have any links to/from them.

protesilaos commented 4 months ago

About this:

If FILE-TYPE is non-nil and different from the current file type, change
the extension of the file.  (Important note: No attempt is made to
modify the content of the file and we only add a new front matter at the
beginning of the file (rather than modify an existing one).

I think it is a good feature to have, though we need to limit it to our file types. Otherwise we get the prompt when trying to rename, say, a PDF.

EDIT: Adding a possible diff.

 denote.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/denote.el b/denote.el
index 79e0500..58449e2 100644
--- a/denote.el
+++ b/denote.el
@@ -2794,7 +2794,8 @@ (defun denote--rename-get-file-info-from-prompts-or-existing (file)
                            signature
                            (format "Rename `%s' with SIGNATURE (empty to remove)" file-in-prompt))))
          ('file-type
-          (setq file-type (denote--valid-file-type (denote-file-type-prompt))))
+          (when (denote-file-is-note-p file)
+            (setq file-type (denote--valid-file-type (denote-file-type-prompt)))))
          ('subdirectory
           (setq directory (file-name-as-directory (denote-subdirectory-prompt))))
          ('date
jeanphilippegg commented 4 months ago

If we keep the status quo, then we need to make sure that no prompt for a change to the front matter happens when we just rename for a signature.

Good catch! It is a good idea to have the signature in the front matter, but I don't think that we need to do it right away. I say we keep the status quo. I made a pull request to arrange the code to only attempt to rewrite (or add) the front matter when it makes sense (ie if the title or keywords have changed).

About this:

      ;; TODO: Offer to regenerate link descriptions in other files on
      ;; rename. Maybe this should be a distinct command.
      (when denote--used-ids
        (puthash id t denote--used-ids))

If we make it part of the renaming process, let us put it behind an opt-in user option. Otherwise it adds more friction. For example, I frequently rename files that do not have any links to/from them.

I actually had no plan to work on this in the short term (or at all). I changed it from TODO to NOTE.

I think it is a good feature to have, though we need to limit it to our file types. Otherwise we get the prompt when trying to rename, say, a PDF.

EDIT: Adding a possible diff.

This is good, but this fix is not enough to make it work with images and PDFs for example.

Though it is still possible to fix it, I recommend that we just remove the new file-type and directory parameters.

First, we never really want to change a file's extension (or type). It only makes sense in our case because we may have created a file of the wrong type and we want to fix it and change the front matter. It is a very specific use case. The general case (even outside Denote) is that we should never modify an extension. I just recommend bringing back denote-change-file-type-and-front-matter to handle this specific case.

Second, changing the directory is not really problematic, but it is more flexible to do this using the standard Emacs command. Our renaming commands would only prompt for a subdirectory of denote-directory. This feature has not been requested either.

For these reasons, I think it makes sense that the renaming commands should only focus on changing the file name components (not the extension or location).