dogears.el icon indicating copy to clipboard operation
dogears.el copied to clipboard

dogears-within-function and dogears--within return incorrect Within value

Open chansey97 opened this issue 7 months ago • 1 comments

Reproduce steps:

  1. Open bug-dogears.el with

    (defun test1 () 1)
    
    (defun test2 () 2)
    
  2. Move cursor to the 1st position of the 2nd line, like |(defun test2 () 2)

  3. M-x dogears-remember

  4. M-x dogear-list

The expect behavior shows:

#  ✓ Relevance  Within                    Line                        Buffer                 Mode            Pos
0  ✓ definition   (defun test2 () 2)  (defun test2 () 2)   bug-dogears.el  emacs-lisp    21 ...

The actual behavior shows:

#  ✓ Relevance  Within                   Line                        Buffer                  Mode            Pos
0  ✓ definition   (defun test1 () 1)  (defun test2 () 2)   bug-dogears.el  emacs-lisp    21 ...

Obviously, there is a problem with the Within value.

The root cause is there are two bugs in calculating Within

  1. The dogears--which-function always returns nil (the 1st bug).

    You can try M-x eval-expression with (dogears--which-function) in the step 3 to check dogears--which-function returns nil.

  2. When dogears--which-function return nil, dogears--place falls back to dogears--within (which has the 2nd bug that causes the unexpected behavior).

    You can try M-x eval-expression with (dogears--within) in the step 3 to check it returns the unexpected Within value.

The suggested patches:

For the 1st bug, let (add-log-current-defun-function nil) in dogears--which-function

(defun dogears--which-function ()
  "Call `which-function' while preventing it from using `add-log-current-defun'."
  (cl-letf (((symbol-function 'add-log-current-defun) #'ignore)
            (add-log-current-defun-function nil)) ; <--- PATCH HERE
    (which-function)))

Frankly, I don't know why you have to ignore add-log-current-defun? Deleting ((symbol-function 'add-log-current-defun) #'ignore) also works. Nevertheless, I follow you convention.

More details see https://github.com/emacs-mirror/emacs/blob/667ca66481c74325f3c8e4391d185ee547fdbb36/lisp/progmodes/which-func.el#L338

Note that to fix the example above only, this patch is enough.

For the 2st bug, add (end-of-defun) to dogears--within.

(defun dogears--within ()
  "Return string representing what the current place is \"within\"."
  (cl-case major-mode
    (Info-mode
     ;; HACK: To make Info buffer records more useful, return nil so the
     ;; bookmark name is used.
     nil)
    (otherwise (ignore-errors
                 (save-excursion
                   (end-of-defun) ; <--- PATCH HERE
                   (beginning-of-defun)
                   (buffer-substring (point-at-bol) (point-at-eol)))))))

Due to the 1st patch, this fallback situation is now rarely encountered. Also, this patch isn't perfect (you know that if the cursor at the last position of defun). Anyway, I add it since most of the time, the cursor to be in the first position of a defun is very common, e.g. xref-find-definitions.

P.s. In order to make dogears better fit my own workflow, I have made many changes to dogears. I would submit separate issues and enhancement patches in the future. So don't care about the initial motivation at the moment.

These are just two isolated bugs.

Thanks.

chansey97 avatar Jul 03 '24 03:07 chansey97