Button-lock-mode conflicts with magit-visit-thing

[lassemaatta]

First of all, this is a really neat library.

The problem: Normally when navigating e.g. the commit log in magit the RET key is bound to magit-visit-thing, which opens the corresponding commit in a new window. I tried creating a config, which would detect Jira ticket identifiers in the commit messages and use button-lock-mode to turn those identifiers into clickable links to Jira. Works great, except now magit-visit-thing fails due to "There is no thing at point that could be visited".

Any idea what might cause this or if there's some workaround?

My very rudimentary I-have-no-idea-what-I'm-doing config is something like:

  (defcustom my-jira-root "https://jira.atlassian.com"
    "Default link to your Jira root."
    :type 'string
    :group 'my-customs)

  (defcustom my-jira-pattern "\\(DEV\\|FOO\\)-[0-9]+"
    "Default pattern for detecting Jira tickets.
  For example, match strings like \"DEV-123\" or \"FOO-1\"."
    :type 'regexp
    :group 'my-customs)

  (use-package button-lock
    :init
    (defun my-create-buttons ()
      (interactive)
      (button-lock-set-button
       my-jira-pattern
       (lambda ()
         (interactive)
         (browse-url (concat my-jira-root
                             "/browse/"
                             (buffer-substring
                              (previous-single-property-change (point) 'mouse-face)
                              (next-single-property-change (point) 'mouse-face)))))
       :face (list 'org-link)
       :mouse-face 'custom-button-mouse
       :keyboard-binding "RET")
      (button-lock-mode))
   ;; I'd like to add various magit-modes here :(
    :hook ((org-mode . my-create-buttons)))

[rolandwalker]

Hi! Does it all work when the button-lock keyboard-binding is not RET ?

[lassemaatta]

Nope, leaving out :keyboard-binding does not help.

I did a bit of investigation wrt. the magit log buffer and it looks like:

• By default, when not using button-lock-mode, the RET key is actually bound to magit-show-commit. If I M-x magit-visit-thing I get the same error as earlier. EDIT: Also, c-h m claims that RET is bound to magit-visit-thing
• If I call that my-create-buttons function, somehow the RET key gets bound to magit-visit-thing.

EDIT: I guess the odd behaviour of magit-visit-thing vs magit-show-commit is somehow related to the way magit rebinds the magit-visit-thing in different keymaps depending on the situation (example) Or at least I think that's somehow related, I'm no emacs expert.

[rolandwalker]

Hm, I still suspect the issue is related to RET, and that we could choose a different key like "j" for JIRA.

I'm looking at the same thing inside Magit as you are in your EDIT:

• https://github.com/magit/magit/blob/b926ab0e051e497e8ed43754cfd2012d23dbfc4c/lisp/magit-mode.el#L326-L331
• https://github.com/magit/magit/blob/b926ab0e051e497e8ed43754cfd2012d23dbfc4c/lisp/magit-mode.el#L416-L423
• https://github.com/magit/magit/blob/b926ab0e051e497e8ed43754cfd2012d23dbfc4c/lisp/magit-log.el#L1154-L1156

magit-visit-thing is a placeholder command whose only purpose is to be remapped. It's hard to reason through.

[rolandwalker]

What does M-x describe-key RET RET tell us, in the commit log?

[lassemaatta]

I tried :keyboard-binding "j" -> no concrete change. Moving the point over a link/button and pressing j opens the link in the browser, but RET still rebinds.

[lassemaatta]

Before activating button-lock:

RET (translated from <return>) runs the command magit-show-commit
(found in magit-commit-section-map), which is an autoloaded
interactive native-compiled Lisp function in ‘magit-diff.el’.

It is bound to <2>, RET and C-<return>.

Afterwards

RET (translated from <return>) runs the command magit-visit-thing
(found in magit-log-mode-map), which is an interactive native-compiled
Lisp function in ‘magit-mode.el’.

It is bound to RET and C-<return>.

[rolandwalker]

I suppose we can reach in and map the key directly:

M-:(define-key magit-commit-section-map (read-kbd-macro "RET") 'magit-show-commit)RET

[lassemaatta]

Sure, I guess that's a good workaround. Thanks for the help sorting this out :)

[lassemaatta]

Just in case someone else encounters this; I ended up with something like this which seems to work ok:

(use-package button-lock
  :init
  (defvar-local my-button nil)
  (defun my-toggle-jira-buttons ()
    (interactive)
    (if (bound-and-true-p button-lock-mode)
        (progn
          (message "Disabling button-lock-mode")
          (button-lock-unset-button my-button)
          (button-lock-mode -1)
          (setq-local my-button nil))
      (progn
        (message "Enabling button-lock-mode")
        (button-lock-mode +1)
        (setq-local my-button
                    (button-lock-set-button
                     my-jira-pattern
                     (lambda ()
                       (interactive)
                       (browse-url (concat my-jira-root
                                           "/browse/"
                                           (buffer-substring
                                            (previous-single-property-change (point) 'mouse-face)
                                            (next-single-property-change (point) 'mouse-face)))))
                     :face (list 'org-link)
                     :mouse-face 'custom-button-mouse
                     :keyboard-binding "RET"))
        ;; Magit tends to forget the `magit-visit-thing' keybindings
        (when (eq major-mode 'magit-status-mode)
          (define-key magit-status-mode-map (read-kbd-macro "RET") 'magit-show-commit))
        (when (eq major-mode 'magit-log-mode)
          (define-key magit-log-mode-map (read-kbd-macro "RET") 'magit-show-commit))
        ;; Make sure the new link style is applied
        (font-lock-fontify-buffer))))
  :hook ((org-mode . my-toggle-jira-buttons)
         (magit-log-mode . my-toggle-jira-buttons)
         (magit-status-mode . my-toggle-jira-buttons)))

(that my-button local thing might not be necessary, but I guess it's nice to do some cleanup with button-lock-unset-button)

[rolandwalker]

Great! I just hope no other keys are affected.