Hello again
Unless I missed it, I didn't see any functions included in the package
to conveniently add or remove a keyword to the current note. Of course,
one could simply edit the front-matter and do a rename based on it, but
it's also nice to work with an interactive prompt.
I wrote the functions below for my personal config, but perhaps they
could be included in the manual?
There are some caveats:
- I'm not sure they use the correct internal Denote functions.
- The functions only work on a buffer visiting a file (so they won't
work on a newly created note that isn't saved yet).
- The filename won't be updated automatically, so users still have to
do denote-rename-file-using-front-matter themselves. I'm not sure how
this can best be automated.
If you feel like these are big issues, perhaps its best not to include
in the manual, but I thought of sharing the code here if others are
interested.
All the best
Elias
#+begin_src emacs-lisp
(defun efls/denote-keyword-add (keywords)
"Add KEYWORDS to current note.
KEYWORDS should be a list of strings."
(interactive
(list (denote-keywords-prompt)))
(unless (denote--current-file-is-note-p)
(user-error "Buffer not visiting a Denote note"))
(let* ((file (buffer-file-name))
(file-type (denote-filetype-heuristics file))
(cur-keywords (denote-retrieve-keywords-value file file-type))
(new-keywords (seq-uniq (append keywords cur-keywords))))
(denote--rewrite-keywords
file
new-keywords
file-type)))
(defun efls/denote-keyword-remove ()
"Prompt for a keyword from current Denote and remove it."
(interactive)
(unless (denote--current-file-is-note-p)
(user-error "Buffer not visiting a Denote note"))
(let* ((file (buffer-file-name))
(file-type (denote-filetype-heuristics file))
(cur-keywords (denote-extract-keywords-from-path file))
(del-keyword (completing-read "Keyword to remove: " cur-keywords))
(new-keywords (delete del-keyword cur-keywords)))
(denote--rewrite-keywords
file
new-keywords
file-type)))
#+end_src
> From: Elias Storms <elias.storms@gmail.com>> Date: Sun, 25 Sep 2022 19:27:47 +0200>>> Hello again
Good day Elias,
> Unless I missed it, I didn't see any functions included in the package> to conveniently add or remove a keyword to the current note. Of course,> one could simply edit the front-matter and do a rename based on it, but> it's also nice to work with an interactive prompt.
You are right. We have the front matter ---> rename, but no interactive
functions of this sort.
> I wrote the functions below for my personal config, but perhaps they> could be included in the manual?
I think yes. If there is demand for them, they could go straight into
denote.el as well. Two questions then:
1. What do you prefer?
2. We are waiting for the paperwork on the copyright assignment, right?
> There are some caveats:> - I'm not sure they use the correct internal Denote functions.
They look fine. The second one could accept a KEYWORD argument. We
just need to move the keyword extraction and prompt inside the
'interactive' spec. To keep it clean, this combination of keyword
extraction and prompt can be its own little function.
> - The functions only work on a buffer visiting a file (so they won't> work on a newly created note that isn't saved yet).
That's okay. We do this in other places as well.
> - The filename won't be updated automatically, so users still have to> do denote-rename-file-using-front-matter themselves. I'm not sure how> this can best be automated.
The functions would need to be modified so that their final part is a
call to 'denote-rename-file-using-front-matter'.
Otherwise the 'denote-rename-file-and-buffer' could be used (again, with
tweaks).
NOTE: I will now make denote--current-file-is-note-p a public function,
so it will be updated to denote-current-file-is-note-p, but there will
be an alias for backward compatibility.
> If you feel like these are big issues, perhaps its best not to include> in the manual, but I thought of sharing the code here if others are> interested.
You did well to share the code. Thank you for doing so!
All the best,
Prot
--
Protesilaos Stavrou
https://protesilaos.com
> From: Protesilaos Stavrou <info@protesilaos.com>> Date: Mon, 26 Sep 2022 05:46:51 +0300> [... 49 lines elided]> NOTE: I will now make denote--current-file-is-note-p a public function,> so it will be updated to denote-current-file-is-note-p, but there will> be an alias for backward compatibility.
Please ignore this part. There is a TODO to review those private
functions and likely remove them. It is better to rely on
'denote-file-is-note-p', which was 'denote--only-note-p'.
(defun denote-file-is-note-p (file)
"Return non-nil if FILE is an actual Denote note.
For our purposes, a note must note be a directory, must satisfy
`file-regular-p', its path must be part of the variable
`denote-directory', it must have a Denote identifier in its name,
and use one of the extensions implied by `denote-file-type'."
(let ((file-name (file-name-nondirectory file)))
(and (not (file-directory-p file))
(file-regular-p file)
(string-prefix-p (denote-directory) (expand-file-name file))
(string-match-p (concat "\\`" denote--id-regexp) file-name)
(denote-file-has-supported-extension-p file))))
--
Protesilaos Stavrou
https://protesilaos.com
Hi Prot
Thanks for the reply.
> I think yes. If there is demand for them, they could go straight into> denote.el as well. Two questions then:>> 1. What do you prefer?
Straight into denote.el has my preference.
> 2. We are waiting for the paperwork on the copyright assignment, right?
I've got the forms to sign for the copyright assignment, so that should
be done soon.
> The functions would need to be modified so that their final part is a> call to 'denote-rename-file-using-front-matter'.
I tried this in my personal config, but then you are prompted a
y-or-no twice (one to save the buffer, another to rename). Perhaps we
need to streamline the process a bit more. I'll have to look into
'denote-rename-file-and-buffer' to see how it could be used to achieve this.
Thanks for the update with 'denote-file-is-note-p'. I've included it as
follows in the -keyword-add and -keyword-remove functions:
(unless (and (buffer-file-name)
(denote-file-is-note-p (buffer-file-name)))
(user-error "Buffer not visiting a Denote note"))
As soon as the copyright assignment is in order, I'll give you an update
on the code I'm using.
Have a nice day
Elias
Protesilaos Stavrou <info@protesilaos.com> writes:
>> From: Elias Storms <elias.storms@gmail.com>>> Date: Sun, 25 Sep 2022 19:27:47 +0200>>>>>> Hello again>> Good day Elias,>>> Unless I missed it, I didn't see any functions included in the package>> to conveniently add or remove a keyword to the current note. Of course,>> one could simply edit the front-matter and do a rename based on it, but>> it's also nice to work with an interactive prompt.>> You are right. We have the front matter ---> rename, but no interactive> functions of this sort.>>> I wrote the functions below for my personal config, but perhaps they>> could be included in the manual?>> I think yes. If there is demand for them, they could go straight into> denote.el as well. Two questions then:>> 1. What do you prefer?> 2. We are waiting for the paperwork on the copyright assignment, right?>>> There are some caveats:>> - I'm not sure they use the correct internal Denote functions.>> They look fine. The second one could accept a KEYWORD argument. We> just need to move the keyword extraction and prompt inside the> 'interactive' spec. To keep it clean, this combination of keyword> extraction and prompt can be its own little function.>>> - The functions only work on a buffer visiting a file (so they won't>> work on a newly created note that isn't saved yet).>> That's okay. We do this in other places as well.>>> - The filename won't be updated automatically, so users still have to>> do denote-rename-file-using-front-matter themselves. I'm not sure how>> this can best be automated.>> The functions would need to be modified so that their final part is a> call to 'denote-rename-file-using-front-matter'.>> Otherwise the 'denote-rename-file-and-buffer' could be used (again, with> tweaks).>> NOTE: I will now make denote--current-file-is-note-p a public function,> so it will be updated to denote-current-file-is-note-p, but there will> be an alias for backward compatibility.>>> If you feel like these are big issues, perhaps its best not to include>> in the manual, but I thought of sharing the code here if others are>> interested.>> You did well to share the code. Thank you for doing so!>> All the best,> Prot
> From: Elias Storms <elias.storms@gmail.com>> Date: Mon, 26 Sep 2022 09:22:41 +0200>>> Hi Prot
Hello Elias,
> Thanks for the reply.
You are welcome!
>> I think yes. If there is demand for them, they could go straight into>> denote.el as well. Two questions then:>>>> 1. What do you prefer?>> Straight into denote.el has my preference.
Very well! Fine by me.
>> 2. We are waiting for the paperwork on the copyright assignment, right?>> I've got the forms to sign for the copyright assignment, so that should> be done soon.
Okay. We will wait for it to be done.
>> The functions would need to be modified so that their final part is a>> call to 'denote-rename-file-using-front-matter'.>> I tried this in my personal config, but then you are prompted a> y-or-no twice (one to save the buffer, another to rename). Perhaps we> need to streamline the process a bit more. I'll have to look into> 'denote-rename-file-and-buffer' to see how it could be used to achieve this.
Okay, it will indeed need to streamlined.
> Thanks for the update with 'denote-file-is-note-p'. I've included it as> follows in the -keyword-add and -keyword-remove functions:>> (unless (and (buffer-file-name)> (denote-file-is-note-p (buffer-file-name)))> (user-error "Buffer not visiting a Denote note"))
Good. By the way, I am adding the finishing touches to the helper
function we covered in the other thread. I will push the change
shortly.
> As soon as the copyright assignment is in order, I'll give you an update> on the code I'm using.
Looking forward to it.
> Have a nice day> Elias
You too!
All the best,
Prot
--
Protesilaos Stavrou
https://protesilaos.com
Hi all
Here's an updated version for the commands to add & remove keywords
interactively.
I've included a denote-rename-file-using-front-matter, but this means
the function asks for confirmation twice (once to save the buffer, once
to do the rename). Not sure whether it can be streamlined (or whether
that's something we want).
I think the denote-keyword-add is pretty much done, but there's two
minor issues remaining with the remove keyword functions. What do you
think, Prot, should we do something to address these issues? Or is it
polished enough like this?
All the best
Elias (aka EFLS)
* Add keyword
We need to take into account the scenario in which there's no keyword yet:
use both an if-let (for values that cannot be nil) and a let form (for
values that can be nil).
#+begin_src emacs-lisp
(defun denote-keyword-add (keywords)
"Add KEYWORDS to current note.
KEYWORDS should be a list of strings.
Note that the file will be renamed based on the updated front matter."
(interactive
(list (denote-keywords-prompt)))
(if-let* ((file (buffer-file-name))
(file-type (denote-filetype-heuristics file)))
(let* ((cur-keywords (denote-retrieve-keywords-value file file-type))
(new-keywords (seq-uniq (append keywords cur-keywords))))
(denote--rewrite-keywords file new-keywords file-type)
(denote-rename-file-using-front-matter file))
(message "Buffer not visiting a Denote file")))
#+end_src
* Remove keyword
Issue 1: when last keyword is removed, there's no frontmatter to update
with, so the denote-rename-file-using-front-matter won't do anything.
Issue 2: when there's no keyword to remove, the interactive prompt still
asks for one (from an empty list)...
#+begin_src emacs-lisp
(defun denote-keyword-remove ()
"Prompt for a keyword from current Denote and remove it.
Note that the file will be renamed based on the updated front matter."
(interactive)
(if-let* ((file (buffer-file-name))
(file-type (denote-filetype-heuristics file)))
(let* ((cur-keywords (denote-extract-keywords-from-path file))
(del-keyword (completing-read "Keyword to remove: " cur-keywords))
(new-keywords (delete del-keyword cur-keywords)))
(denote--rewrite-keywords file new-keywords file-type)
(denote-rename-file-using-front-matter file))
(message "Buffer not visiting a Denote file")))
#+end_src
Protesilaos Stavrou <info@protesilaos.com> writes:
>> From: Elias Storms <elias.storms@gmail.com>>> Date: Mon, 26 Sep 2022 09:22:41 +0200>>>>>> Hi Prot>> Hello Elias,>>> Thanks for the reply.>> You are welcome!>>>> I think yes. If there is demand for them, they could go straight into>>> denote.el as well. Two questions then:>>>>>> 1. What do you prefer?>>>> Straight into denote.el has my preference.>> Very well! Fine by me.>>>> 2. We are waiting for the paperwork on the copyright assignment, right?>>>> I've got the forms to sign for the copyright assignment, so that should>> be done soon.>> Okay. We will wait for it to be done.>>>> The functions would need to be modified so that their final part is a>>> call to 'denote-rename-file-using-front-matter'.>>>> I tried this in my personal config, but then you are prompted a>> y-or-no twice (one to save the buffer, another to rename). Perhaps we>> need to streamline the process a bit more. I'll have to look into>> 'denote-rename-file-and-buffer' to see how it could be used to achieve this.>> Okay, it will indeed need to streamlined.>>> Thanks for the update with 'denote-file-is-note-p'. I've included it as>> follows in the -keyword-add and -keyword-remove functions:>>>> (unless (and (buffer-file-name)>> (denote-file-is-note-p (buffer-file-name)))>> (user-error "Buffer not visiting a Denote note"))>> Good. By the way, I am adding the finishing touches to the helper> function we covered in the other thread. I will push the change> shortly.>>> As soon as the copyright assignment is in order, I'll give you an update>> on the code I'm using.>> Looking forward to it.>>> Have a nice day>> Elias>> You too!>> All the best,> Prot
> From: Elias Storms <elias.storms@gmail.com>> Date: Sun, 2 Oct 2022 14:32:49 +0200>> Hi all
Hello Elias,
> Here's an updated version for the commands to add & remove keywords> interactively.
Thanks for sharing! Note that we will eventually need the FSF paperwork
to proceed with those. Though not for your patch about visiting
backlinks (I replied to the other thread).
> I've included a denote-rename-file-using-front-matter, but this means> the function asks for confirmation twice (once to save the buffer, once> to do the rename). Not sure whether it can be streamlined (or whether> that's something we want).
Okay, we will need to refine that part.
> I think the denote-keyword-add is pretty much done, but there's two> minor issues remaining with the remove keyword functions. What do you> think, Prot, should we do something to address these issues? Or is it> polished enough like this?
Right now I am too tired to be effective, so I might miss something. I
say we work on this project as soon as you get the copyright assignment
done. I guess it will be done some time this week.
Some minor comments below.
> * Add keyword>> We need to take into account the scenario in which there's no keyword yet:> use both an if-let (for values that cannot be nil) and a let form (for> values that can be nil).
This clarification of the additional 'let*' form can go into the source
code. Otherwise we might hastily try to merge those into the preceding
'if-let*'.
> #+begin_src emacs-lisp> (defun denote-keyword-add (keywords)> "Add KEYWORDS to current note.> KEYWORDS should be a list of strings.>> Note that the file will be renamed based on the updated front matter."> (interactive> (list (denote-keywords-prompt)))> (if-let* ((file (buffer-file-name))> (file-type (denote-filetype-heuristics file)))> (let* ((cur-keywords (denote-retrieve-keywords-value file file-type))> (new-keywords (seq-uniq (append keywords cur-keywords))))> (denote--rewrite-keywords file new-keywords file-type)> (denote-rename-file-using-front-matter file))> (message "Buffer not visiting a Denote file")))> #+end_src>> * Remove keyword>> Issue 1: when last keyword is removed, there's no frontmatter to update> with, so the denote-rename-file-using-front-matter won't do anything.
Perhaps we need to review denote-rename-file-using-front-matter. Right
now it will not act on an empty keywords' front matter entry. Do we
want it to behave this way?
> Issue 2: when there's no keyword to remove, the interactive prompt still> asks for one (from an empty list)...
I guess you mean your 'del-keywords'? This could check if
'cur-keywords' is non-nil. Whereas now it is called unconditionally.
> #+begin_src emacs-lisp> (defun denote-keyword-remove ()> "Prompt for a keyword from current Denote and remove it.>> Note that the file will be renamed based on the updated front matter."> (interactive)> (if-let* ((file (buffer-file-name))> (file-type (denote-filetype-heuristics file)))> (let* ((cur-keywords (denote-extract-keywords-from-path file))> (del-keyword (completing-read "Keyword to remove: " cur-keywords))> (new-keywords (delete del-keyword cur-keywords)))> (denote--rewrite-keywords file new-keywords file-type)> (denote-rename-file-using-front-matter file))> (message "Buffer not visiting a Denote file")))> #+end_src
All the best,
Prot
--
Protesilaos Stavrou
https://protesilaos.com
Hi Prot
The FSF copyright assignment is finally countersigned! So here I am with
a new version of the functions to add and remove keywords. See below.
As I said before, there's one issue still with the remove function (as
explained in the comment and discussed before):
denote-rename-file-using-front-matter doesn't work when there are no
keywords. This could lead to issues as the keyword will still be in the
filename. Is there any other way to rename the file?
As to where this should be included in denote.el: I'll leave that up to
you. Feel free to alter the code how you see fit.
All the best
Elias (aka EFLS)
(defun denote-keyword-add (keywords)
"Add KEYWORDS to current note.
KEYWORDS should be a list of strings, or is prompted for
interactively.
Note that the file will be renamed based on the new front matter."
(interactive
(list (denote-keywords-prompt)))
;; A combination of if-let and let, as we need to take into account
;; the scenario in which there are no keywords yet.
(if-let* ((file (buffer-file-name))
(file-check (denote-file-is-note-p file))
(file-type (denote-filetype-heuristics file)))
(let* ((cur-keywords (denote-retrieve-keywords-value file file-type))
(new-keywords (seq-uniq (append keywords cur-keywords))))
(denote--rewrite-keywords file new-keywords file-type)
(denote-rename-file-using-front-matter file t))
(message "Buffer not visiting a Denote file")))
(defun denote-keyword-remove ()
"Prompt for a keyword from current Denote and remove it.
Note that the file will be renamed based on the updated front matter."
(interactive)
(if-let* ((file (buffer-file-name))
(file-check (denote-file-is-note-p file))
(file-type (denote-filetype-heuristics file)))
(if-let* ((cur-keywords (denote-retrieve-keywords-value file file-type))
(del-keyword (completing-read
"Keyword to remove: "
cur-keywords
nil
t))
(new-keywords (delete del-keyword cur-keywords)))
(progn
(denote--rewrite-keywords file new-keywords file-type)
(denote-rename-file-using-front-matter file t))
(message "No keywords to remove from current note"))
;; MINOR ISSUE: When last keyword is removed, there's no
;; frontmatter to update with, so
;; denote-rename-file-using-front-matter won't do anything
;; (and the keyword won't be removed from the filename,
;; leading to issues down the line).
(message "Buffer not visiting a Denote file")))
> From: Elias Storms <elias.storms@gmail.com>> Date: Mon, 10 Oct 2022 20:33:02 +0200>> Hi Prot
Good day Elias,
> The FSF copyright assignment is finally countersigned! So here I am with> a new version of the functions to add and remove keywords. See below.
Wonderful!
> As I said before, there's one issue still with the remove function (as> explained in the comment and discussed before):> denote-rename-file-using-front-matter doesn't work when there are no> keywords. This could lead to issues as the keyword will still be in the> filename. Is there any other way to rename the file?>> As to where this should be included in denote.el: I'll leave that up to> you. Feel free to alter the code how you see fit.>> All the best> Elias (aka EFLS)> [... 48 lines elided]
I will review your code later today, as I am committed to another task
right now.
I shall merge it with you as the commit author. I will also make any
other requisite changes to the existing code to facilitate the inclusion
of this new functionality.
More to follow. Thank you!
Prot
--
Protesilaos Stavrou
https://protesilaos.com
> From: Protesilaos Stavrou <info@protesilaos.com>> Date: Tue, 11 Oct 2022 10:19:27 +0300> [... 27 lines elided]> I will review your code later today, as I am committed to another task> right now.
Hello again Elias,
I made some minor tweaks to what you provided. Please find the current
version further below. Before merging this, I want to work on the
"MINOR ISSUE", because it (i) might cause problems and (ii) will not
look nice if we leave it as-is. I am still not sure how best to
approach this though.
- One option is to rewrite the function 'denote-retrieve-keywords-value'
or check the return value of 'denote--keywords-value-reverse-function'.
- Another would be to revise 'denote-rename-file-using-front-matter' so
that it do not throw an error when no front matter is available.
The latter seems wrong prima facie, but I have not thought it through.
Maybe we can have a more granular approach of using a 'cond' instead of
the current all-or-nothing way we implement the 'if-let*'. I need more
time with this. There may still be something that eludes me. Ideas are
welcome.
Your code with my minor tweaks:
;;;###autoload
(defun denote-keyword-add (keywords)
"Prompt for KEYWORDS to add to the current note's front matter.
When called from Lisp, KEYWORDS is a list of strings.
Rename the file without further prompt so that its name includes
the KEYWORDS in the front matter."
(interactive (list (denote-keywords-prompt)))
;; A combination of if-let and let, as we need to take into account
;; the scenario in which there are no keywords yet.
(if-let* ((file (buffer-file-name))
((denote-file-is-note-p file))
(file-type (denote-filetype-heuristics file)))
(let* ((cur-keywords (denote-retrieve-keywords-value file file-type))
(new-keywords (seq-uniq (append keywords cur-keywords))))
(denote--rewrite-keywords file new-keywords file-type)
(denote-rename-file-using-front-matter file t))
(message "Buffer not visiting a Denote file")))
;;;###autoload
(defun denote-keyword-remove ()
"Prompt for a keyword in current note and remove it.
Keywords are retrieved from the file's front matter.
Rename the file without further prompt so that its name matches
the keywords in the front matter."
(declare (interactive-only t))
(interactive)
(if-let* ((file (buffer-file-name))
((denote-file-is-note-p file))
(file-type (denote-filetype-heuristics file)))
(if-let* ((cur-keywords (denote-retrieve-keywords-value file file-type))
(del-keyword (completing-read "Keyword to remove: " cur-keywords nil t))
(new-keywords (delete del-keyword cur-keywords)))
(progn
(denote--rewrite-keywords file new-keywords file-type)
(denote-rename-file-using-front-matter file t))
(message "No keywords to remove from current note"))
;; MINOR ISSUE: When last keyword is removed, there's no
;; frontmatter to update with, so
;; denote-rename-file-using-front-matter won't do anything
;; (and the keyword won't be removed from the filename,
;; leading to issues down the line).
(message "Buffer not visiting a Denote file")))
All the best,
Prot
--
Protesilaos Stavrou
https://protesilaos.com
> From: Protesilaos Stavrou <info@protesilaos.com>> Date: Tue, 11 Oct 2022 19:28:07 +0300> [... 11 lines elided]> I made some minor tweaks to what you provided. Please find the current> version further below. Before merging this, I want to work on the> "MINOR ISSUE", because it (i) might cause problems and (ii) will not> look nice if we leave it as-is. I am still not sure how best to> approach this though.>> - One option is to rewrite the function 'denote-retrieve-keywords-value'> or check the return value of 'denote--keywords-value-reverse-function'.>> - Another would be to revise 'denote-rename-file-using-front-matter' so> that it do not throw an error when no front matter is available.>> The latter seems wrong prima facie, but I have not thought it through.> Maybe we can have a more granular approach of using a 'cond' instead of> the current all-or-nothing way we implement the 'if-let*'. I need more> time with this. There may still be something that eludes me. Ideas are> welcome.> [... 50 lines elided]
Good day Elias!
I think I have addressed the underlying constraint in the aforementioned
"MINOR ISSUE".
commit 63a2b2145307ad9575be93920cbe83b7f83e649e
Author: Protesilaos Stavrou <info@protesilaos.com>
Date: Wed Oct 12 06:37:09 2022 +0300
Make renaming remove keywords if empty
The idea is an empty "keywords" entry in the front matter should mean
that the "__KEYWORDS" component of the title should not be present.
We already allow this, such as if the 'denote-prompts' is configured
to not have a 'keywords' prompt (check its doc string).
THIS NEEDS MORE TESTING TO BE SURE THERE ARE NO REGRESSIONS.
denote.el | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
We need to test this thoroughly. Then we can proceed to add your
commands.
All the best,
Prot
--
Protesilaos Stavrou
https://protesilaos.com
> From: Protesilaos Stavrou <info@protesilaos.com>> Date: Wed, 12 Oct 2022 06:42:14 +0300> [... 26 lines elided]> Good day Elias!>> I think I have addressed the underlying constraint in the aforementioned> "MINOR ISSUE".>> commit 63a2b2145307ad9575be93920cbe83b7f83e649e> Author: Protesilaos Stavrou <info@protesilaos.com>> Date: Wed Oct 12 06:37:09 2022 +0300>> Make renaming remove keywords if empty>> The idea is an empty "keywords" entry in the front matter should mean> that the "__KEYWORDS" component of the title should not be present.> We already allow this, such as if the 'denote-prompts' is configured> to not have a 'keywords' prompt (check its doc string).>> THIS NEEDS MORE TESTING TO BE SURE THERE ARE NO REGRESSIONS.>> denote.el | 9 +++++++--> 1 file changed, 7 insertions(+), 2 deletions(-)>> We need to test this thoroughly. Then we can proceed to add your> commands.
Just to follow-up with the natural extension to this. Now both the
title and the keywords will not error out if they are empty. The error
occurs (correctly) when they are not present at all.
commit 522896fe8c42a7109602b40ad43ce6c4782e52a3
Author: Protesilaos Stavrou <info@protesilaos.com>
Date: Wed Oct 12 08:47:14 2022 +0300
Make renaming remove title if empty
This is the same idea as with commit 63a2b21. NEEDS THOROUGH TESTING.
denote.el | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
--
Protesilaos Stavrou
https://protesilaos.com
Hi Prot
Many thanks for your code review and the other changes.
Unfortunately, I still run into some issues when testing the add &
remove keyword commands.
(1) When the keyword frontmatter line is empty (i.e., contains no
keywords), you can't add a new keyword.
The reason: denote-retrieve-keywords-value returns an empty string ("")
when the keywords line is empty, causing seq-uniq for new keywords to
fail.
Perhaps the easiest fix is to change denote-retrieve-keywords-value so
that it will return nil when the keyword line is empty?
(2) Similar to (1), an error occurs when attempting to remove a keyword
when the keyword line is empty.
Similar to the above, I think.
(3) When there is only one keyword, attempting to remove it will
state "No keywords to remove from current note"
The reason: the value of new-keywords will be nil, causing the if-let*
to pick the else option.
The fix: include a separate let for new-keywords (see below).
In the code below, I've fixed (3), but I haven't addressed the other
two. What do you think is the best way to solve them?
All the best
Elias
;;;###autoload
(defun denote-keyword-add (keywords)
"Prompt for KEYWORDS to add to the current note's front matter.
When called from Lisp, KEYWORDS is a list of strings.
Rename the file without further prompt so that its name includes
the KEYWORDS in the front matter."
(interactive (list (denote-keywords-prompt)))
;; A combination of if-let and let, as we need to take into account
;; the scenario in which there are no keywords yet.
(if-let* ((file (buffer-file-name))
((denote-file-is-note-p file))
(file-type (denote-filetype-heuristics file)))
(let* ((cur-keywords (denote-retrieve-keywords-value file file-type))
(new-keywords (seq-uniq (append keywords cur-keywords))))
(denote--rewrite-keywords file new-keywords file-type)
(denote-rename-file-using-front-matter file t))
(message "Buffer not visiting a Denote file")))
;;;###autoload
(defun denote-keyword-remove ()
"Prompt for a keyword in current note and remove it.
Keywords are retrieved from the file's front matter.
Rename the file without further prompt so that its name matches
the keywords in the front matter."
(declare (interactive-only t))
(interactive)
(if-let* ((file (buffer-file-name))
((denote-file-is-note-p file))
(file-type (denote-filetype-heuristics file)))
(if-let* ((cur-keywords (denote-retrieve-keywords-value file file-type))
(del-keyword (completing-read "Keyword to remove: " cur-keywords nil t)))
(let ((new-keywords (delete del-keyword cur-keywords)))
(denote--rewrite-keywords file new-keywords file-type)
(denote-rename-file-using-front-matter file t))
(message "No keywords to remove from current note"))
;; MINOR ISSUE: When last keyword is removed, there's no
;; frontmatter to update with, so
;; denote-rename-file-using-front-matter won't do anything
;; (and the keyword won't be removed from the filename,
;; leading to issues down the line).
(message "Buffer not visiting a Denote file")))
> From: Elias Storms <elias.storms@gmail.com>> Date: Wed, 12 Oct 2022 14:09:07 +0200>> Hi Prot
Hello Elias,
> Many thanks for your code review and the other changes.
You are welcome!
> Unfortunately, I still run into some issues when testing the add &> remove keyword commands.>> [... 18 lines elided]> In the code below, I've fixed (3), but I haven't addressed the other> two. What do you think is the best way to solve them?> [... 48 lines elided]
You are right. These commands will need to be tweaked further.
The changes I made were meant to address shortcomings in the existing
code base. With the various permutations of 'denote-prompts', we can
have all those valid file names:
DATE.EXT
DATE--TITLE.EXT
DATE__KEYWORDS.EXT
DATE--TITLE__KEYWORDS.EXT (this is the standard)
But 'denote-rename-file-using-front-matter' did not cover the scenario
where a file name switches between those patterns. So we first need to
ensure this is done consistently (I am still testing) before building on
it.
Though, again, you are right with the issues you identified.
All the best,
Prot
--
Protesilaos Stavrou
https://protesilaos.com
> From: Elias Storms <elias.storms@gmail.com>> Date: Wed, 12 Oct 2022 14:09:07 +0200>> Hi Prot
Good day Elias,
> Many thanks for your code review and the other changes.
You are welcome!
> Unfortunately, I still run into some issues when testing the add &> remove keyword commands.>> (1) When the keyword frontmatter line is empty (i.e., contains no> keywords), you can't add a new keyword.> The reason: denote-retrieve-keywords-value returns an empty string ("")> when the keywords line is empty, causing seq-uniq for new keywords to> fail.> Perhaps the easiest fix is to change denote-retrieve-keywords-value so> that it will return nil when the keyword line is empty?>> (2) Similar to (1), an error occurs when attempting to remove a keyword> when the keyword line is empty.> Similar to the above, I think.>> (3) When there is only one keyword, attempting to remove it will> state "No keywords to remove from current note"> The reason: the value of new-keywords will be nil, causing the if-let*> to pick the else option.> The fix: include a separate let for new-keywords (see below).>> In the code below, I've fixed (3), but I haven't addressed the other> two. What do you think is the best way to solve them?
I reworte the commands to address all three points. For 3, I took a
different approach than what you had, with the intent to suggest being
less verbose in such cases. What do you think?
;;;###autoload
(defun denote-keyword-add (keywords)
"Prompt for KEYWORDS to add to the current note's front matter.
When called from Lisp, KEYWORDS is a list of strings.
Rename the file without further prompt so that its name includes
the KEYWORDS in the front matter."
(interactive (list (denote-keywords-prompt)))
;; A combination of if-let and let, as we need to take into account
;; the scenario in which there are no keywords yet.
(if-let* ((file (buffer-file-name))
((denote-file-is-note-p file))
(file-type (denote-filetype-heuristics file)))
(let* ((cur-keywords (denote-retrieve-keywords-value file file-type))
(new-keywords (if (string-blank-p cur-keywords)
keywords
(seq-uniq (append keywords cur-keywords)))))
(denote--rewrite-keywords file new-keywords file-type)
(denote-rename-file-using-front-matter file t))
(message "Buffer not visiting a Denote file")))
;;;###autoload
(defun denote-keyword-remove ()
"Prompt for a keyword in current note and remove it.
Keywords are retrieved from the file's front matter.
Rename the file without further prompt so that its name matches
the keywords in the front matter."
(declare (interactive-only t))
(interactive)
(if-let* ((file (buffer-file-name))
((denote-file-is-note-p file))
(file-type (denote-filetype-heuristics file)))
(when-let* ((cur-keywords (denote-retrieve-keywords-value file file-type))
((or (listp cur-keywords) (not (string-blank-p cur-keywords))))
(del-keyword (completing-read "Keyword to remove: " cur-keywords nil t)))
(denote--rewrite-keywords file (delete del-keyword cur-keywords) file-type)
(denote-rename-file-using-front-matter file t))
(message "Buffer not visiting a Denote file")))
All the best,
Prot
--
Protesilaos Stavrou
https://protesilaos.com
Hi Prot
> I reworte the commands to address all three points. For 3, I took a> different approach than what you had, with the intent to suggest being> less verbose in such cases. What do you think?
Thanks for this next iteration. Nice solution to all of the issues, I've
tested it (albeit briefly) and it works very well!
I'm keeping it in my personal config for now, but I think it makes a
nice addition to the main codebase. Perhaps we need to use the function
for a couple of days to see whether there are any remaining issues, and
after this trial period you could include it? Or whatever you think is
best.
Have a nice day
Elias
> From: Elias Storms <elias.storms@gmail.com>> Date: Fri, 14 Oct 2022 14:55:03 +0200>> Hi Prot
Hello Elias,
>> I reworte the commands to address all three points. For 3, I took a>> different approach than what you had, with the intent to suggest being>> less verbose in such cases. What do you think?>> Thanks for this next iteration. Nice solution to all of the issues, I've> tested it (albeit briefly) and it works very well!
Very well!
> I'm keeping it in my personal config for now, but I think it makes a> nice addition to the main codebase. Perhaps we need to use the function> for a couple of days to see whether there are any remaining issues, and> after this trial period you could include it? Or whatever you think is> best.
I think they should be added directly to denote.el. We are in the
current development cycle, after all. If something needs to be tweaked,
we are ready to do it.
I will prepare the commit which adds those two commands with you as its
author. Is this okay? Then I will update the manual in a separate
commit.
All the best,
Prot
--
Protesilaos Stavrou
https://protesilaos.com
> I will prepare the commit which adds those two commands with you as its> author. Is this okay? Then I will update the manual in a separate> commit.
Very well!
Elias
> From: Elias Storms <elias.storms@gmail.com>> Date: Fri, 14 Oct 2022 15:10:32 +0200>>> I will prepare the commit which adds those two commands with you as its>> author. Is this okay? Then I will update the manual in a separate>> commit.>> Very well!>> Elias
I just added them:
commit 91c25fbeed64da0bb483a835a69485271547c68b
Author: Elias Storms <elias.storms@gmail.com>
Date: Fri Oct 14 19:08:05 2022 +0300
Add commands to add/remove keywords and rename file
denote.el | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
commit 6c26b18e7a3c9edae49f49283532f21203e105b0
Author: Protesilaos Stavrou <info@protesilaos.com>
Date: Fri Oct 14 19:09:33 2022 +0300
Document commands to add/remove keywords
See commit 91c25fb by Elias Storms. Elias has assigned copyright to
the Free Software Foundation.
Also read the relevant thread on the mailing list:
<https://lists.sr.ht/~protesilaos/denote/%3Cm24jwvpbt2.fsf%40MBA21.fritz.box%3E#%3Cm28rlik0tc.fsf@MBA21.fritz.box%3E>.
README.org | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
I think part of the ongoing development effort is to make
'denote-keyword-remove' use a 'completing-read-multiple' prompt the way
its counterpart does. I will give it a try tomorrow morning.
--
Protesilaos Stavrou
https://protesilaos.com
Great! One additional issue I just ran into: when adding a keyword when
there is already one or more keywords, I get en error:
string-blank-p: Wrong type argument: stringp, ("test")
The reason is that 'cur-keywords' is a list, so 'string-blank-p' doesn't
work. I think the solution is to work with a cond.
If it is correct that denote-retrieve-keywords-value returns:
- an empty string if the keywords line is empty
- a list of strings if there are one or more keywords
then the solution could be to check against those two cases, but use a
listp first.
See below for a possible fix (that seems to work, but not tested
thoroughly). Only the new-keywords in the let* has changed.
All the best
Elias
;;;###autoload
(defun denote-keyword-add (keywords)
"Prompt for KEYWORDS to add to the current note's front matter.
When called from Lisp, KEYWORDS is a list of strings.
Rename the file without further prompt so that its name includes
the KEYWORDS in the front matter."
(interactive (list (denote-keywords-prompt)))
;; A combination of if-let and let, as we need to take into account
;; the scenario in which there are no keywords yet.
(if-let* ((file (buffer-file-name))
((denote-file-is-note-p file))
(file-type (denote-filetype-heuristics file)))
(let* ((cur-keywords (denote-retrieve-keywords-value file file-type))
(new-keywords (cond ((listp cur-keywords)
(seq-uniq (append keywords cur-keywords)))
((string-blank-p cur-keywords)
keywords))))
(denote--rewrite-keywords file new-keywords file-type)
(denote-rename-file-using-front-matter file t))
(message "Buffer not visiting a Denote file")))
> From: Elias Storms <elias.storms@gmail.com>> Date: Fri, 14 Oct 2022 19:25:20 +0200>> Great! One additional issue I just ran into: when adding a keyword when> there is already one or more keywords, I get en error:>> string-blank-p: Wrong type argument: stringp, ("test")
Ah, good catch!
> The reason is that 'cur-keywords' is a list, so 'string-blank-p' doesn't> work. I think the solution is to work with a cond.>> If it is correct that denote-retrieve-keywords-value returns:> - an empty string if the keywords line is empty> - a list of strings if there are one or more keywords> then the solution could be to check against those two cases, but use a> listp first.>> See below for a possible fix (that seems to work, but not tested> thoroughly). Only the new-keywords in the let* has changed.
I did it a bit differently, by using an 'and' clause, like:
(and (stringp cur-keywords) (string-blank-p cur-keywords))
I simply did it this way because it is the smaller diff. What do you
think?
--
Protesilaos Stavrou
https://protesilaos.com
Hi Prot
> I simply did it this way because it is the smaller diff. What do you> think?
Excellent, much more elegant solution. Thanks for the quick fix.
Elias
> From: Elias Storms <elias.storms@gmail.com>> Date: Sat, 15 Oct 2022 13:58:04 +0200>> Hi Prot>>> I simply did it this way because it is the smaller diff. What do you>> think?>> Excellent, much more elegant solution. Thanks for the quick fix.
You are welcome!
I implemented two more changes:
commit ed1402ad30d5877762eb5f53ee43c65298e8b1e1
Author: Protesilaos Stavrou <info@protesilaos.com>
Date: Sat Oct 15 15:08:46 2022 +0300
BREAKING: Use plural in new command symbols
See commits 6c26b18, 91c25fb.
README.org | 10 +++++-----
denote.el | 4 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
commit ced969aecf711b109856a7a8b51a68d166411090
Author: Protesilaos Stavrou <info@protesilaos.com>
Date: Sat Oct 15 15:36:25 2022 +0300
Make denote-keywords-remove use completing-read-multiple
This makes the interface consistent with denote-keywords-add and, by
extension, with every command that leverages the denote-keywords-prompt.
README.org | 5 +++--
denote.el | 27 +++++++++++++++++++++------
2 files changed, 24 insertions(+), 8 deletions(-)
--
Protesilaos Stavrou
https://protesilaos.com