From: Noboru Ota <me@nobiot.com>
To: ~protesilaos/denote@lists.sr.ht
Cc: Protesilaos Stavrou <info@protesilaos.com>
Subject: Re: [PATCH] Display context of identifier in backlinks buffer with
 xref
In-Reply-To: <87o7tzl1zv.fsf@protesilaos.com>
Date: Fri, 28 Oct 2022 17:24:36 +0200
Message-ID: <86ilk47yyz.fsf@nobiot.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="

--=-=-=
Content-Type: text/plain

Hi Prot,

I wanted to refactor the implementation, so I went ahead and created
patches (attached).  I have elaborated on the intent and implementation
detail in the commit message.

I will be on two-week vacation from Saturday tomorrow; this is the main
reason why I really wanted to send all the patches to you.

I realize that you would be busy with the updates on Modus theme -- it's
the theme I use and love.  Thank you :).  Please treat these patches as
another discussion starter.  There should be more elegant way to get the
backlinks logic closer to the built-in way with Xref.  But I would like
to believe that the current way is not so bad.  At least functionally, I
have tested them on my Emacs 28.1 for all the four cases of backlinks
(context on/off and two commands 'denote-link-backlinks' and
'denote-link-find-backlink').

I will not carry my laptop to the vacation but I should be able to read
emails.  If there is anything I can/should do, I will see what I can do
in mid November.

Warm regards,
nobiot



--=-=-=
Content-Type: text/x-diff
Content-Disposition: attachment;
 filename=0001-Add-no-backlink-user-error-denote-link-find-backlink.patch
Content-Description: patch 1/2

From e8a4be9f9e33a6babe37664e9bcacdfa48e55d2b Mon Sep 17 00:00:00 2001
From: Noboru Ota <me@nobiot.com>
Date: Fri, 28 Oct 2022 15:59:56 +0200
Subject: [PATCH 1/2] Add "no backlink" user-error 'denote-link-find-backlink'

'denote-link-find-backlink' emits a user-error when no backlink is found
for the note.  The same should be done for 'denote-link-find-backlink'.
---
 denote.el | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/denote.el b/denote.el
index 2b0f787..53ac5f9 100644
--- a/denote.el
+++ b/denote.el
@@ -2451,14 +2451,15 @@ whitespace-only), insert an ID-ONLY link."
 
 Like `denote-link-find-file', but select backlink to follow."
   (interactive)
-  (when-let* ((file (buffer-file-name))
+  (if-let* ((file (buffer-file-name))
               (id (denote-retrieve-filename-identifier file))
               (files (denote--retrieve-files-in-xrefs
                       (denote--retrieve-process-grep id))))
-    (find-file
-     (denote-get-path-by-id
-      (denote-extract-id-from-string
-       (denote-link--find-file-prompt files))))))
+      (find-file
+       (denote-get-path-by-id
+        (denote-extract-id-from-string
+         (denote-link--find-file-prompt files))))
+    (user-error "No links found in the current buffer")))
 
 ;;;###autoload
 (defun denote-link-after-creating (&optional id-only)
-- 
2.34.1


--=-=-=
Content-Type: text/x-diff
Content-Disposition: attachment;
 filename=0002-Refactor-backlink-xref-functions.patch
Content-Description: patch 2/2

From 6e4930cb3527e3be7e47373214420da280e36165 Mon Sep 17 00:00:00 2001
From: Noboru Ota <me@nobiot.com>
Date: Fri, 28 Oct 2022 16:36:27 +0200
Subject: [PATCH 2/2] Refactor backlink xref functions

The main intent of this change is to remove 'denote-xref--insert-xrefs',
which was meant to be a slight modification to the built-in function
'xref--insert-xrefs'.  This way, it is intended that future maintenance
work for Denote will be lighter and more in line with the changes to the
built-in Xref library.

Additionally, refactoring is done for 'denote--retrieve-xrefs'. The
intent was to fulfill requirements (A) (B) and (1) (2) described below
in a way that avoids duplication of work -- an attempt to follow the DRY
(Don't Repeat Yourself) principle.

The function name 'denote--retrieve-xrefs' indicates that it should be
generic that should return all the xrefs as retrieved by the Xref
function (A); however, the current usage is only to retrieve backlinks
for a specific file, by excluding a relevant element from the xref-alist
(B). This is used by two different commands: (1) 'denote-link-backlinks'
and (2) 'denote-link-find-backlink'; they have different requirements
for the output format: (1) requires relative file names and (2) requires
an absolute file names.

There can be possibly further refactoring.  That is now left to future iteration.

More detail of change to each altered function follows.

* denote.el

(denote--retrieve-xrefs): Add an optional argument FILE. It is meant to
cater to the different requirements of (A) and (B) above.

(denote--retrieve-process-grep): This function is now used only by
'denote-link-backlinks' and refactored to transform GROUP from absolute
file name to relative one for `denote-directory`. This is requirement
(1) above for display purposes of the backlinks' buffer As the doc
string of 'xref--insert-xrefs' notes that GROUP in xref-alist is for
"decoration purposes" and it is also processed in the same way in
'xref--analyze', this should not be a problem.

(denote-link-find-backlink): Refactor to avoid function
'(denote--retrieve-process-grep)' to retrieve xref-alist for requirement
(2) above.

(denote-link--prepare-backlinks): Refactor to use 'xref--insert-xrefs'
directly. As GROUP in xref-alist (GROUP is its key) now carries relative
file name for display purposes, the original logic to insert a list of
file names is also refactored to remove the duplicate work of function
'denote-get-file-name-relative-to-denote-directory'.  It is now done
centrally in 'denote--retrieve-process-grep' for the purpose of
backlinks' buffer (with or without the context).

(denote-xref--insert-xrefs): Remove this function completely.
---
 denote.el | 107 ++++++++++++++++++++----------------------------------
 1 file changed, 40 insertions(+), 67 deletions(-)

diff --git a/denote.el b/denote.el
index 53ac5f9..8b6abaf 100644
--- a/denote.el
+++ b/denote.el
@@ -1152,13 +1152,21 @@ Run `denote-desluggify' on title if the extraction is sucessful."
       title
     (denote-retrieve-filename-title file)))
 
-(defun denote--retrieve-xrefs (identifier)
+(defun denote--retrieve-xrefs (identifier &optional file)
   "Return xrefs of IDENTIFIER in variable `denote-directory'.
-The xrefs are returned as an alist."
-  (xref--alistify
-   (xref-matches-in-files identifier (denote-directory-text-only-files))
-   (lambda (x)
-     (xref-location-group (xref-item-location x)))))
+The xrefs are returned as an alist of the form:
+
+((GROUP . (XREF ...)) ...)
+
+GROUP is an absolute file name as retrieved by Xref facility.
+
+When FILE is present, remove its GROUP from the alist."
+  (let ((alist
+         (xref--alistify
+          (xref-matches-in-files identifier (denote-directory-text-only-files))
+          (lambda (x)
+            (xref-location-group (xref-item-location x))))))
+    (if file (assoc-delete-all file alist) alist)))
 
 (defun denote--retrieve-files-in-xrefs (xref-alist)
   "Return sorted, deduplicated file names from XREF-ALIST."
@@ -1167,9 +1175,26 @@ The xrefs are returned as an alist."
    #'string-lessp))
 
 (defun denote--retrieve-process-grep (identifier)
-  "Process lines matching IDENTIFIER and return list of xref-alist."
-  (assoc-delete-all (buffer-file-name)
-                    (denote--retrieve-xrefs identifier)))
+  "Process lines matching IDENTIFIER and return list of xref-alist.
+
+The alist is of the form ((GROUP . (XREF ...)) ...).
+
+The alist excludes GROUP for the file that current buffer is
+visiting so that only its backlinks are colleced.
+
+In addition, GROUP is a transformed to filename relative to
+`denote-directory', which is the string displayed in the
+backlinks' buffer."
+  ;;; This `mapcar' form is doing what function `xref--analyze' would
+  ;;; do.  `xref--analyze' can be flexibly configured but is not used
+  ;;; directly here because it assumes that the current directory is in
+  ;;; a "project" as defined in project.el.  For Denote, this is not the
+  ;;; case (at least as at the time of this writing).
+  (mapcar
+   (lambda (xref)
+     (cons (denote-get-file-name-relative-to-denote-directory (car xref))
+           (cdr xref)))
+   (denote--retrieve-xrefs identifier (buffer-file-name))))
 
 ;;;; New note
 
@@ -2452,9 +2477,10 @@ whitespace-only), insert an ID-ONLY link."
 Like `denote-link-find-file', but select backlink to follow."
   (interactive)
   (if-let* ((file (buffer-file-name))
-              (id (denote-retrieve-filename-identifier file))
-              (files (denote--retrieve-files-in-xrefs
-                      (denote--retrieve-process-grep id))))
+            (id (denote-retrieve-filename-identifier file))
+            (files
+             (denote--retrieve-files-in-xrefs
+              (denote--retrieve-xrefs id (buffer-file-name)))))
       (find-file
        (denote-get-path-by-id
         (denote-extract-id-from-string
@@ -2659,11 +2685,10 @@ Use optional TITLE for a prettier heading."
                   (heading (format "Backlinks to %S (%s)" title id))
                   (l (length heading)))
         (insert (format "%s\n%s\n\n" heading (make-string l ?-))))
-      ;;; We could have a user option to use the current backlink buffer
       (if denote-backlinks-show-context
-          (denote-xref--insert-xrefs xref-alist)
+          (xref--insert-xrefs xref-alist)
         (mapc (lambda (x)
-                (insert (denote-get-file-name-relative-to-denote-directory (car x)))
+                (insert (car x))
                 (make-button (line-beginning-position) (line-end-position) :type 'denote-link-backlink-button)
                 (newline))
               xref-alist))
@@ -2675,58 +2700,6 @@ Use optional TITLE for a prettier heading."
                       (denote-link--prepare-backlinks id xref-alist title)))))
     (denote-link--display-buffer buf)))
 
-(defun denote-xref--insert-xrefs (xref-alist)
-  "Insert XREF-ALIST in the current buffer.
-XREF-ALIST is of the form ((GROUP . (XREF ...)) ...), where GROUP
-is a string for decoration purposes and XREF is an `xref-item'
-object.  This function is a slightly modified version of the
-built-in function `xref--insert-xrefs'."
-  (require 'compile) ; For the compilation faces.
-  (cl-loop for (group . xrefs) in xref-alist
-           for max-line = (cl-loop for xref in xrefs
-                                   maximize (xref-location-line
-                                             (xref-item-location xref)))
-           for line-format = (and max-line
-                                  (format "%%%dd: " (1+ (floor (log max-line 10)))))
-           with item-text-props = (list 'mouse-face 'highlight
-                                        'keymap xref--button-map
-                                        'help-echo
-                                        (concat "mouse-2: display in another window, "
-                                                "RET or mouse-1: follow reference"))
-           with prev-group = nil
-           with prev-line = nil
-           do
-           (xref--insert-propertized '(face xref-file-header xref-group t)
-                                     (denote-get-file-name-relative-to-denote-directory group) "\n")
-           (dolist (xref xrefs)
-             (pcase-let (((cl-struct xref-item summary location) xref))
-               (let* ((line (xref-location-line location))
-                      (prefix
-                       (cond
-                        ((not line) "  ")
-                        ((and (equal line prev-line)
-                              (equal prev-group group))
-                         "")
-                        (t (propertize (format line-format line)
-                                       'face 'xref-line-number)))))
-                 ;; Render multiple matches on the same line, together.
-                 (when (and (equal prev-group group)
-                            (or (null line)
-                                (not (equal prev-line line))))
-                   (insert "\n"))
-                 (xref--insert-propertized (nconc (list 'xref-item xref)
-                                                  item-text-props)
-                                           prefix summary)
-                 (setq prev-line line
-                       prev-group group))))
-           (insert "\n"))
-  (add-to-invisibility-spec '(ellipsis . t))
-  (save-excursion
-    (goto-char (point-min))
-    (while (= 0 (forward-line 1))
-      (xref--apply-truncation)))
-  (run-hooks 'xref-after-update-hook))
-
 ;;;###autoload
 (defun denote-link-backlinks ()
   "Produce a buffer with backlinks to the current note.
-- 
2.34.1


--=-=-=--