From 0786f73669baa468596b29bd0f0d26956770676d Mon Sep 17 00:00:00 2001 From: Taro Sato Date: Wed, 24 Jan 2024 12:40:38 -0800 Subject: [PATCH] fix: write unlinked references regex to a temp file Node titles with special characters (single quotes, dollar signs, etc.) break the ripgrep command because the regex pattern is passed through the shell. This causes silent failures that show up as unlinked references not being present for a given node. This change writes the regex pattern to a temp file and uses ripgrep's --file option instead of shell command line. `shell-quote-argument` is replaced with `regexp-quote` since we're no longer passing through shell. Wrapped in unwind-protect for cleanup. Fix: #2407 Close: #2408 --- org-roam-mode.el | 37 ++++++++++++++++++++++++++----------- tests/test-org-roam-mode.el | 4 ++-- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/org-roam-mode.el b/org-roam-mode.el index 7d8018b..d6d6927 100644 --- a/org-roam-mode.el +++ b/org-roam-mode.el @@ -658,17 +658,26 @@ This is the ROW within FILE." (end-of-line) (point))))) -(defun org-roam-unlinked-references--rg-command (titles) - "Return the ripgrep command searching for TITLES." +(defun org-roam-unlinked-references--rg-command (titles temp-file) + "Return the ripgrep command searching for TITLES using TEMP-FILE for pattern. +This avoids shell escaping issues by writing the pattern to a file instead +of passing it directly through the shell command line." + ;; Write pattern to temp file to avoid shell escaping issues with quotes, + ;; spaces, and other special characters in titles + (with-temp-file temp-file + (insert "\\[([^[]]++|(?R))*\\]" + (mapconcat (lambda (title) + ;; Use regexp-quote instead of shell-quote-argument + ;; since we're writing a regex pattern, not a shell argument + (format "|(\\b%s\\b)" (regexp-quote title))) + titles ""))) + (concat "rg --follow --only-matching --vimgrep --pcre2 --ignore-case " (mapconcat (lambda (glob) (concat "--glob " glob)) (org-roam--list-files-search-globs org-roam-file-extensions) " ") - (format " '\\[([^[]]++|(?R))*\\]%s' " - (mapconcat (lambda (title) - (format "|(\\b%s\\b)" (shell-quote-argument title))) - titles "")) - (shell-quote-argument org-roam-directory))) + " --file " (shell-quote-argument temp-file) " " + (shell-quote-argument (expand-file-name org-roam-directory)))) (defun org-roam-unlinked-references-section (node) "The unlinked references section for NODE. @@ -679,9 +688,13 @@ References from FILE are excluded." (shell-command-to-string "rg --pcre2-version")))) (let* ((titles (cons (org-roam-node-title node) (org-roam-node-aliases node))) - (rg-command (org-roam-unlinked-references--rg-command titles)) - (results (split-string (shell-command-to-string rg-command) "\n")) - f row col match) + ;; Create temp file for the regex pattern + (temp-file (make-temp-file "org-roam-rg-pattern-")) + (rg-command (org-roam-unlinked-references--rg-command titles temp-file))) + ;; Use unwind-protect to ensure temp file cleanup even if errors occur + (unwind-protect + (let* ((results (split-string (shell-command-to-string rg-command) "\n")) + f row col match) (magit-insert-section (unlinked-references) (magit-insert-heading "Unlinked References:") (dolist (line results) @@ -705,7 +718,9 @@ References from FILE are excluded." (org-roam-fontify-like-in-org-mode (org-roam-unlinked-references-preview-line f row)) "\n")))))) - (insert ?\n))))) + (insert ?\n))) + ;; Clean up temp file - this runs even if an error occurs above + (delete-file temp-file))))) (provide 'org-roam-mode) ;;; org-roam-mode.el ends here diff --git a/tests/test-org-roam-mode.el b/tests/test-org-roam-mode.el index cacfce4..fdef91c 100644 --- a/tests/test-org-roam-mode.el +++ b/tests/test-org-roam-mode.el @@ -30,9 +30,9 @@ (setq org-roam-directory "/tmp/org roam")) (it "returns the correct rg command for unlinked references" - (expect (org-roam-unlinked-references--rg-command '("foo" "bar")) + (expect (org-roam-unlinked-references--rg-command '("foo" "bar") "/tmp/regex") :to-equal - "rg --follow --only-matching --vimgrep --pcre2 --ignore-case --glob \"*.org\" --glob \"*.org.gpg\" --glob \"*.org.age\" '\\[([^[]]++|(?R))*\\]|(\\bfoo\\b)|(\\bbar\\b)' /tmp/org\\ roam"))) + "rg --follow --only-matching --vimgrep --pcre2 --ignore-case --glob \"*.org\" --glob \"*.org.gpg\" --glob \"*.org.age\" --file /tmp/regex /tmp/org\\ roam"))) (provide 'test-org-roam-mode)