Make sure org-roam-node-from-title-or-alias won't mess up with the match-data #1766
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tch-data It lies in between org-in-regexp and replace-match. In some situations, like when the link looks like roam:a s'b, it changes the match-data.
jethrokuan
reviewed
Aug 12, 2021
| @@ -628,7 +628,7 @@ Assumes that the cursor was put where the link is." | |||
| (goto-char (org-element-property :begin link)) | |||
| (when (and (org-in-regexp org-link-any-re 1) | |||
| (string-equal type "roam") | |||
| (setq node (org-roam-node-from-title-or-alias path))) | |||
| (setq node (save-match-data (org-roam-node-from-title-or-alias path)))) | |||
There was a problem hiding this comment.
In this case, I'll move the save-match-data into org-roam-node-from-title-or-alias.
There was a problem hiding this comment.
I don't mind, but IIUC, the idiomatic way of doing it in emacs appears
to put save-match-data in the function that needs the match data to be
saved.
At least, this it what I understand by reading at the documentation of
save-match-data:
so ‘save-match-data’ should normally be used to save *your* match data
rather than your caller’s match data.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It lies in between org-in-regexp and replace-match. In some situations, like
when the link looks like roam:a s'b, it changes the match-data.
Motivation for this change
Get file file test.txt
It is an elisp file, but github won't allow the extension .el...
It
[[roam:a s'b]]org-roam-link-replace-allThis simulates the behavior when you create a note and use the completion mecanism to add a link to another note.
Run this with emacs -Q --load test.txt, you will get the following error
org-roam-link-replace-at-point: Args out of range: 0, 0Try removing the "'" character, so that the first file has the title "a sb" and the roam link is now
[[roam:a sb]]. Now the test works.Digging a little bit, I found out that, when the title is "a s'b"
org-roam-link-replace-at-pointcallsorg-roam-node-from-title-or-aliasorg-roam-node-from-title-or-aliascallsorg-roam-db-queryorg-roam-db-querycallsemacsqlandorg-roam-dbemacsqlandorg-roam-dbchange the match-dataI did not look further. May be some other place of the code changes the match-data.
But I can tell that nesting
org-roam-link-replace-at-pointinsidesave-match-datafixes the issue.Moreover, I guess it is a good practice to ensure that the code between anything that sets the match data (
org-in-regexpin this example) and anything that relies on this match-data (replace-matchin this example) does not change the match-data.