From 57ad26288823d739cad8d5ab2fb36474b9d96483 Mon Sep 17 00:00:00 2001 From: Guilherme Thomazi Bonicontro Date: Wed, 6 May 2026 11:51:54 +0200 Subject: [PATCH 1/4] fix: note indexing, wikilink creation, inbox review, and tree refresh --- grove-capture.el | 20 ++------- grove-core.el | 18 +++++++- grove-inbox.el | 8 +++- grove-link.el | 4 +- grove-tree.el | 19 +++++++- tests/grove-review-test.el | 89 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 136 insertions(+), 22 deletions(-) create mode 100644 tests/grove-review-test.el diff --git a/grove-capture.el b/grove-capture.el index 65a3b72..0d4b1b5 100644 --- a/grove-capture.el +++ b/grove-capture.el @@ -30,6 +30,8 @@ (require 'grove-core) +(declare-function grove--unique-path "grove-core" (directory filename)) + (defvar grove-capture-mode-map (let ((map (make-sparse-keymap))) (define-key map (kbd "C-c C-c") #'grove-capture-finalize) @@ -42,22 +44,6 @@ :lighter " Grove-Capture" :keymap grove-capture-mode-map) -(defun grove-capture--unique-path (directory filename) - "Return a unique file path in DIRECTORY for FILENAME. -Appends a numeric suffix if the file already exists." - (let ((base (file-name-sans-extension filename)) - (ext (file-name-extension filename t)) - (path (expand-file-name filename directory))) - (if (not (file-exists-p path)) - path - (let ((n 1)) - (while (file-exists-p - (setq path (expand-file-name - (concat base (format "-%d" n) ext) - directory))) - (setq n (1+ n))) - path)))) - ;;;###autoload (defun grove-capture () "Open a buffer for quick note capture. @@ -88,7 +74,7 @@ Type freely, then press \\[grove-capture-finalize] to save or (title (string-trim (car lines))) (body (string-join (cdr lines) "\n")) (filename (concat (grove--sanitize-filename title) ".org")) - (path (grove-capture--unique-path (grove--inbox-path) filename))) + (path (grove--unique-path (grove--inbox-path) filename))) (with-temp-file path (insert "#+title: " title "\n") (unless (string-empty-p body) diff --git a/grove-core.el b/grove-core.el index 3fee498..4d6296b 100644 --- a/grove-core.el +++ b/grove-core.el @@ -123,13 +123,29 @@ Matches #tag-style markers while ignoring Org keyword lines such as (make-directory path t)) (file-name-as-directory path))) +(defun grove--unique-path (directory filename) + "Return a unique file path in DIRECTORY for FILENAME. +Appends a numeric suffix if the file already exists." + (let ((base (file-name-sans-extension filename)) + (ext (file-name-extension filename t)) + (path (expand-file-name filename directory))) + (if (not (file-exists-p path)) + path + (let ((n 1)) + (while (file-exists-p + (setq path (expand-file-name + (concat base (format "-%d" n) ext) + directory))) + (setq n (1+ n))) + path)))) + (defun grove--parse-note (file) "Parse an org FILE and return a metadata plist. Returns (:title TITLE :tags TAGS :links LINKS :mtime MTIME)." (let ((mtime (file-attribute-modification-time (file-attributes file))) title tags links) (with-temp-buffer - (insert-file-contents file nil 0 4096) + (insert-file-contents file) ;; Extract #+title: (goto-char (point-min)) (when (re-search-forward "^#\\+title:\\s-*\\(.+\\)" nil t) diff --git a/grove-inbox.el b/grove-inbox.el index abde606..5765fe3 100644 --- a/grove-inbox.el +++ b/grove-inbox.el @@ -102,6 +102,7 @@ NOTES is a list of (TITLE . PATH)." (grove--ensure-directory) (grove--refresh-cache) (let* ((untagged (grove-inbox--untagged-notes)) + (unlinked (grove-inbox--unlinked-notes)) (buf (get-buffer-create grove-inbox-buffer-name))) (with-current-buffer buf (grove-inbox-mode) @@ -115,9 +116,14 @@ NOTES is a list of (TITLE . PATH)." (grove-inbox--insert-section (format "Untagged (%d)" (length untagged)) untagged) + (grove-inbox--insert-section + (format "No backlinks (%d)" (length unlinked)) + unlinked) (goto-char (point-min)))) (switch-to-buffer buf) - (message "Found %d untagged note(s)" (length untagged)))) + (message "Found %d untagged and %d unlinked note(s)" + (length untagged) + (length unlinked)))) (defun grove-inbox-close () "Close the inbox review buffer." diff --git a/grove-link.el b/grove-link.el index 6d5e128..084650c 100644 --- a/grove-link.el +++ b/grove-link.el @@ -30,6 +30,8 @@ (require 'grove-core) +(declare-function grove--unique-path "grove-core" (directory filename)) + ;;;; Faces (defface grove-link @@ -122,7 +124,7 @@ If the note doesn't exist, offer to create it." (find-file path) (if (y-or-n-p (format "Note \"%s\" not found. Create it? " title)) (let* ((filename (concat (grove--sanitize-filename title) ".org")) - (path (expand-file-name filename grove-directory))) + (path (grove--unique-path grove-directory filename))) (find-file path) (insert "#+title: " title "\n\n")) (message "Link not followed"))))) diff --git a/grove-tree.el b/grove-tree.el index 7b4cfac..72297d2 100644 --- a/grove-tree.el +++ b/grove-tree.el @@ -305,8 +305,23 @@ Directories come first, then files. Hidden files are excluded." (erase-buffer) (setq grove-tree--ewoc (ewoc-create #'grove-tree--print "" "")) - (dolist (node (grove-tree--list-entries grove-directory 0)) - (ewoc-enter-last grove-tree--ewoc node))))))) + (grove-tree--populate grove-directory 0 nil)))))) + +(defun grove-tree--populate (directory depth parent-node) + "Insert DIRECTORY entries at DEPTH after PARENT-NODE. +If a directory is marked expanded in `grove-tree--expanded', recursively +insert its visible descendants." + (let ((prev parent-node)) + (dolist (node (grove-tree--list-entries directory depth)) + (setq prev (if prev + (ewoc-enter-after grove-tree--ewoc prev node) + (ewoc-enter-last grove-tree--ewoc node))) + (when (and (grove-tree-node-directory-p node) + (gethash (grove-tree-node-path node) grove-tree--expanded)) + (setq prev (grove-tree--populate (grove-tree-node-path node) + (1+ depth) + prev)))) + prev)) ;;;; Current file tracking diff --git a/tests/grove-review-test.el b/tests/grove-review-test.el new file mode 100644 index 0000000..a7596f2 --- /dev/null +++ b/tests/grove-review-test.el @@ -0,0 +1,89 @@ +;;; grove-review-test.el --- Regression tests for review findings -*- lexical-binding: t -*- + +;; Copyright 2026 Guilherme Thomazi Bonicontro + +;;; Commentary: + +;; Regression coverage for issues found during code review. + +;;; Code: + +(require 'ert) +(require 'grove-core) +(require 'grove-link) +(require 'grove-inbox) +(require 'grove-tree) +(require 'subr-x) + +(ert-deftest grove-parse-note-reads-beyond-4kb () + (let ((file (make-temp-file "grove-large" nil ".org" + (concat "#+title: Large note\n\n" + (make-string 5000 ?a) + "\n#late-tag\n[[Late Link]]\n")))) + (unwind-protect + (let ((meta (grove--parse-note file))) + (should (equal (plist-get meta :tags) '("late-tag"))) + (should (equal (plist-get meta :links) '("Late Link")))) + (delete-file file)))) + +(ert-deftest grove-link-follow-creates-unique-file-on-filename-collision () + (let* ((grove-directory (make-temp-file "grove-vault" t)) + (existing (expand-file-name "foo.org" grove-directory))) + (unwind-protect + (progn + (with-temp-file existing + (insert "#+title: Existing\n\n")) + (cl-letf (((symbol-function 'grove-link--resolve) (lambda (_title) nil)) + ((symbol-function 'y-or-n-p) (lambda (&rest _) t))) + (grove-link-follow "Foo!") + (should (buffer-file-name)) + (should (string= (file-name-nondirectory (buffer-file-name)) "foo-1.org")) + (should (string= (buffer-string) "#+title: Foo!\n\n"))) + (when (buffer-live-p (current-buffer)) + (kill-buffer (current-buffer))) + (with-temp-buffer + (insert-file-contents existing) + (should (string= (buffer-string) "#+title: Existing\n\n")))) + (delete-directory grove-directory t)))) + +(ert-deftest grove-inbox-review-renders-unlinked-section () + (let ((grove-directory (make-temp-file "grove-vault" t))) + (unwind-protect + (let ((grove--cache (make-hash-table :test #'equal))) + (puthash "/tmp/a.org" (list :title "A" :tags nil) grove--cache) + (puthash "/tmp/b.org" (list :title "B" :tags '("tag")) grove--cache) + (cl-letf (((symbol-function 'grove--refresh-cache) #'ignore) + ((symbol-function 'grove--ensure-directory) #'ignore) + ((symbol-function 'grove-inbox--unlinked-notes) + (lambda () '(("B" . "/tmp/b.org"))))) + (grove-inbox-review) + (with-current-buffer grove-inbox-buffer-name + (should (string-match-p "No backlinks (1)" (buffer-string))) + (should (string-match-p "B" (buffer-string)))) + (kill-buffer grove-inbox-buffer-name))) + (delete-directory grove-directory t)))) + +(ert-deftest grove-tree-refresh-rebuilds-expanded-children () + (let* ((grove-directory (make-temp-file "grove-vault" t)) + (subdir (expand-file-name "sub" grove-directory)) + (file (expand-file-name "note.org" subdir))) + (unwind-protect + (progn + (make-directory subdir) + (with-temp-file file + (insert "#+title: Note\n")) + (with-current-buffer (get-buffer-create grove-tree-buffer-name) + (grove-tree-mode) + (clrhash grove-tree--expanded) + (puthash subdir t grove-tree--expanded) + (grove-tree-refresh) + (let ((items nil)) + (ewoc-map (lambda (node) (push (grove-tree-node-name node) items)) + grove-tree--ewoc) + (should (member "sub" items)) + (should (member "note" items)))) + (kill-buffer grove-tree-buffer-name)) + (delete-directory grove-directory t)))) + +(provide 'grove-review-test) +;;; grove-review-test.el ends here From 37857fe9c65d72b77afe7eca2aefdebce3186e7a Mon Sep 17 00:00:00 2001 From: Guilherme Thomazi Bonicontro Date: Wed, 6 May 2026 12:05:18 +0200 Subject: [PATCH 2/4] fix: colon-title links --- grove-core.el | 14 ++++++++++++-- grove-link.el | 24 +++++++++++++----------- tests/grove-review-test.el | 20 ++++++++++++++++++++ 3 files changed, 45 insertions(+), 13 deletions(-) diff --git a/grove-core.el b/grove-core.el index 4d6296b..f29b71d 100644 --- a/grove-core.el +++ b/grove-core.el @@ -71,6 +71,16 @@ Each value is a plist with keys :title :tags :links :mtime.") "\\(?:^\\|[^[:alnum:]_#]\\)#\\([[:alnum:]_][[:alnum:]_/-]*\\)\\b" "Regexp matching inline hashtags in note content.") +(defconst grove--org-link-protocol-regexp + "\\`[[:alpha:]][[:alnum:]+.-]*:" + "Regexp matching an Org-style link protocol prefix.") + +(defun grove--org-link-target-p (target) + "Return non-nil if TARGET looks like a standard Org link target. +This excludes Grove wikilinks such as note titles containing colons." + (and (string-match-p grove--org-link-protocol-regexp target) + (not (string-match-p "[[:space:]]" target)))) + (defun grove--collect-inline-tags () "Return inline hashtags from the current buffer. Matches #tag-style markers while ignoring Org keyword lines such as @@ -159,8 +169,8 @@ Returns (:title TITLE :tags TAGS :links LINKS :mtime MTIME)." (goto-char (point-min)) (while (re-search-forward "\\[\\[\\([^]]+\\)\\]\\]" nil t) (let ((link (match-string 1))) - ;; Skip standard org links (contain a colon protocol) - (unless (string-match-p ":" link) + ;; Skip standard org links like http: or file: + (unless (grove--org-link-target-p link) (push link links))))) (list :title (or title (file-name-sans-extension (file-name-nondirectory file))) diff --git a/grove-link.el b/grove-link.el index 084650c..a0f7a2d 100644 --- a/grove-link.el +++ b/grove-link.el @@ -42,9 +42,9 @@ ;;;; Font-lock (defconst grove-link--regexp - "\\[\\[\\([^]:\n]+\\)\\]\\]" + "\\[\\[\\([^]\n]+\\)\\]\\]" "Regexp matching grove wikilinks. -Matches [[text]] but not [[protocol:text]] (standard org links).") +Matches [[text]], including note titles containing colons.") (defvar grove-link--keymap (let ((map (make-sparse-keymap))) @@ -57,15 +57,17 @@ Matches [[text]] but not [[protocol:text]] (standard org links).") "Font-lock matcher for grove wikilinks up to LIMIT. Adds the grove-link face and a clickable keymap." (while (re-search-forward grove-link--regexp limit t) - (let ((start (match-beginning 0)) - (end (match-end 0))) - (add-text-properties - start end - (list 'face 'grove-link - 'mouse-face 'highlight - 'keymap grove-link--keymap - 'help-echo (format "grove: %s" (match-string 1)) - 'grove-link-target (match-string 1))))) + (let ((target (match-string 1))) + (unless (grove--org-link-target-p target) + (let ((start (match-beginning 0)) + (end (match-end 0))) + (add-text-properties + start end + (list 'face 'grove-link + 'mouse-face 'highlight + 'keymap grove-link--keymap + 'help-echo (format "grove: %s" target) + 'grove-link-target target)))))) nil) (defun grove-link-setup-font-lock () diff --git a/tests/grove-review-test.el b/tests/grove-review-test.el index a7596f2..52d9220 100644 --- a/tests/grove-review-test.el +++ b/tests/grove-review-test.el @@ -26,6 +26,26 @@ (should (equal (plist-get meta :links) '("Late Link")))) (delete-file file)))) +(ert-deftest grove-parse-note-keeps-colon-titles-as-wikilinks () + (let ((file (make-temp-file "grove-colon" nil ".org" + "#+title: Link test\n\n[[Project: Alpha]]\n[[https://example.com]]\n"))) + (unwind-protect + (should (equal (plist-get (grove--parse-note file) :links) + '("Project: Alpha"))) + (delete-file file)))) + +(ert-deftest grove-link-fontify-allows-colons-in-note-titles () + (with-temp-buffer + (insert "[[Project: Alpha]] [[https://example.com]]") + (goto-char (point-min)) + (grove-link--fontify (point-max)) + (goto-char (point-min)) + (search-forward "[[Project: Alpha]]") + (should (equal (get-text-property (match-beginning 0) 'grove-link-target) + "Project: Alpha")) + (search-forward "[[https://example.com]]") + (should-not (get-text-property (match-beginning 0) 'grove-link-target)))) + (ert-deftest grove-link-follow-creates-unique-file-on-filename-collision () (let* ((grove-directory (make-temp-file "grove-vault" t)) (existing (expand-file-name "foo.org" grove-directory))) From df5df271361377076a21519bfad28dc9f73c025d Mon Sep 17 00:00:00 2001 From: Guilherme Thomazi Bonicontro Date: Wed, 6 May 2026 12:59:23 +0200 Subject: [PATCH 3/4] fix: ripgrep error handling --- grove-backlink.el | 53 ++++++++++++++++++++++++-------------- grove-tree.el | 30 +++++++++++++++++++++ tests/grove-review-test.el | 39 ++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 20 deletions(-) diff --git a/grove-backlink.el b/grove-backlink.el index 5101225..a9351c7 100644 --- a/grove-backlink.el +++ b/grove-backlink.el @@ -29,10 +29,14 @@ ;;; Code: (require 'grove-core) +(require 'subr-x) (defconst grove-backlink-buffer-name "*grove-backlinks*" "Name of the backlinks buffer.") +(defconst grove-backlink-ripgrep-executable "rg" + "Ripgrep executable used for backlink search.") + ;;;; Core (defun grove-backlink--title-for-file (file) @@ -47,28 +51,37 @@ Uses the cache if available, otherwise the filename." "Return a list of backlink results for TITLE. Each result is a plist (:file :line :context) found via ripgrep." (grove--ensure-directory) + (unless (executable-find grove-backlink-ripgrep-executable) + (user-error "Ripgrep not found. Install `%s` and ensure it is on your PATH" + grove-backlink-ripgrep-executable)) (let* ((pattern (format "\\[\\[%s\\]\\]" (regexp-quote title))) - (cmd (format "rg --no-heading --line-number --context 1 --glob='*.org' %s %s" - (shell-quote-argument pattern) - (shell-quote-argument grove-directory))) - (output (shell-command-to-string cmd)) + (args (list "--no-heading" "--line-number" "--context" "1" + "--glob=*.org" pattern grove-directory)) results current-file) - (dolist (line (split-string output "\n" t)) - (cond - ;; Context separator - ((string-match-p "^--$" line)) - ;; Match line: file:line:content - ((string-match "^\\(.+\\.org\\):\\([0-9]+\\):\\(.*\\)$" line) - (let ((file (match-string 1 line)) - (lnum (string-to-number (match-string 2 line))) - (context (string-trim (match-string 3 line)))) - ;; Skip self-references - (unless (and current-file - (string= file current-file)) - (push (list :file file :line lnum :context context) - results)))) - ;; Context line: file-line-content - ((string-match "^\\(.+\\.org\\)-\\([0-9]+\\)-\\(.*\\)$" line)))) + (with-temp-buffer + (let ((exit-code (apply #'process-file grove-backlink-ripgrep-executable + nil t nil args))) + (unless (memq exit-code '(0 1)) + (user-error "Ripgrep failed (exit %s): %s" + exit-code + (string-trim (buffer-string))))) + (goto-char (point-min)) + (dolist (line (split-string (buffer-string) "\n" t)) + (cond + ;; Context separator + ((string-match-p "^--$" line)) + ;; Match line: file:line:content + ((string-match "^\\(.+\\.org\\):\\([0-9]+\\):\\(.*\\)$" line) + (let ((file (match-string 1 line)) + (lnum (string-to-number (match-string 2 line))) + (context (string-trim (match-string 3 line)))) + ;; Skip self-references + (unless (and current-file + (string= file current-file)) + (push (list :file file :line lnum :context context) + results)))) + ;; Context line: file-line-content + ((string-match "^\\(.+\\.org\\)-\\([0-9]+\\)-\\(.*\\)$" line))))) (setq current-file (buffer-file-name)) ;; Filter out self-references (cl-remove-if diff --git a/grove-tree.el b/grove-tree.el index 72297d2..b8d26f9 100644 --- a/grove-tree.el +++ b/grove-tree.el @@ -96,6 +96,9 @@ Requires a Nerd Font to be installed and active." (defconst grove-tree-buffer-name "*grove-tree*" "Name of the tree sidebar buffer.") +(defvar grove-tree--tracking-enabled nil + "Non-nil when Grove is tracking the current file across window changes.") + (defun grove-tree--indent-string (depth) "Return an indent guide string for DEPTH levels of nesting." (if (zerop depth) @@ -339,6 +342,31 @@ insert its visible descendants." (goto-char pos) (hl-line-highlight)))))))) +(defun grove-tree--track-current-file (&rest _) + "Update the tree highlight to match the currently selected Grove file." + (when grove-tree--tracking-enabled + (let ((file (buffer-file-name (window-buffer (selected-window))))) + (when (grove-file-p file) + (grove-tree--set-current-file file))))) + +(defun grove-tree--enable-tracking () + "Enable tree updates when the selected window changes." + (unless grove-tree--tracking-enabled + (add-hook 'window-selection-change-functions + #'grove-tree--track-current-file) + (add-hook 'buffer-list-update-hook + #'grove-tree--track-current-file) + (setq grove-tree--tracking-enabled t))) + +(defun grove-tree--disable-tracking () + "Disable tree updates for current file tracking." + (when grove-tree--tracking-enabled + (remove-hook 'window-selection-change-functions + #'grove-tree--track-current-file) + (remove-hook 'buffer-list-update-hook + #'grove-tree--track-current-file) + (setq grove-tree--tracking-enabled nil))) + ;;;; Mode (defvar grove-tree-mode-map @@ -386,11 +414,13 @@ insert its visible descendants." (no-delete-other-windows . t))))))) (when win (window-preserve-size win t t)) + (grove-tree--enable-tracking) buf))) (defun grove-tree-close () "Close the tree sidebar." (interactive) + (grove-tree--disable-tracking) (let ((win (get-buffer-window grove-tree-buffer-name))) (when win (delete-window win))) diff --git a/tests/grove-review-test.el b/tests/grove-review-test.el index 52d9220..d388225 100644 --- a/tests/grove-review-test.el +++ b/tests/grove-review-test.el @@ -10,6 +10,7 @@ (require 'ert) (require 'grove-core) +(require 'grove-backlink) (require 'grove-link) (require 'grove-inbox) (require 'grove-tree) @@ -105,5 +106,43 @@ (kill-buffer grove-tree-buffer-name)) (delete-directory grove-directory t)))) +(ert-deftest grove-tree-tracks-current-file-via-hooks () + (let* ((grove-directory (make-temp-file "grove-vault" t)) + (file-a (expand-file-name "a.org" grove-directory)) + (file-b (expand-file-name "b.org" grove-directory))) + (unwind-protect + (progn + (with-temp-file file-a + (insert "#+title: A\n")) + (with-temp-file file-b + (insert "#+title: B\n")) + (with-current-buffer (get-buffer-create grove-tree-buffer-name) + (grove-tree-mode) + (setq grove-tree--ewoc t)) + (cl-letf (((symbol-function 'ewoc-refresh) (lambda (&rest _) nil)) + ((symbol-function 'hl-line-highlight) (lambda (&rest _) nil))) + (grove-tree--enable-tracking) + (find-file file-a) + (grove-tree--track-current-file) + (with-current-buffer grove-tree-buffer-name + (should (equal grove-tree--current-file file-a))) + (find-file file-b) + (grove-tree--track-current-file) + (with-current-buffer grove-tree-buffer-name + (should (equal grove-tree--current-file file-b)))) + (grove-tree--disable-tracking) + (kill-buffer grove-tree-buffer-name) + (when (buffer-file-name) + (kill-buffer (current-buffer)))) + (delete-directory grove-directory t)))) + +(ert-deftest grove-backlink-find-errors-when-ripgrep-is-missing () + (let ((grove-directory (make-temp-file "grove-vault" t))) + (unwind-protect + (cl-letf (((symbol-function 'executable-find) (lambda (&rest _) nil))) + (should-error (grove-backlink--find "Note") + :type 'user-error)) + (delete-directory grove-directory t)))) + (provide 'grove-review-test) ;;; grove-review-test.el ends here From 83688267ac1754eb890dad27045a573fab39a076 Mon Sep 17 00:00:00 2001 From: Guilherme Thomazi Bonicontro Date: Thu, 7 May 2026 08:49:17 +0200 Subject: [PATCH 4/4] chore: update author email --- AUTHORS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AUTHORS b/AUTHORS index 11dee2d..42f4e35 100644 --- a/AUTHORS +++ b/AUTHORS @@ -1,2 +1,2 @@ Jonathan Chu -Guilherme Thomazi Bonicontro +Guilherme Thomazi Bonicontro