Eliminate global advice #8
Description
Greetings and thank you for your package. I was interested in trying this to facilitate on-line devdocs.io browsing vs. the off-line only ability of https://github.com/astoff/devdocs.el which does work well.
I was disappointed to see that the implementation globally advises eww functions that I don't want advised outside the context of your package. Also disappointed to see that when disabling your minor mode, it does not remove the advice. These are a double whammy against the implementation.
The eldoc-documentation-functions advice at least is buffer local so that's generally okay--except when the global default "*eww" buffer is reused for pages unrelated to devdocs.io. Not sure how many people this affects.
I've written a working example of how you can improve the implementation so it doesn't globally pollute, and still achieves its goals. Check out the below for the methods.
I didn't really get as far as exercising the package, though, because global pollution is a non-starter for me. I was pleased to see that no-littering has your package under control via devdocs-browser-cache-directory. I can provide more feedback in the future. Let me know.
Create a simple text.html file in your home directory and copy/paste into your *scratch* buffer and run it.
(progn
(defvar recenter-needed nil)
(setq eww-after-render-hook nil)
(let ((link-keymap (make-sparse-keymap)) ; obv would be keymaps in your name space
(image-link-keymap (make-sparse-keymap)))
(cl-labels
((external-rendering-functions ()
(append
shr-external-rendering-functions
`((a . ,#'filter-tag-a))))
(follow-link (&optional external mouse-event) ; obv your own defun
(interactive)
(let ((shr-external-rendering-functions
(external-rendering-functions))
(recenter-needed t))
(eww-follow-link external mouse-event)))
(recenter-after-render () ; obv your own defun which you have
(when recenter-needed
(recenter)))
(filter-tag-a (dom) ; obv your own defun which you need
(let ((start (point))
(href (dom-attr dom 'href))
(text (car (dom-children dom))))
;; Amend the link as needed.
(when href
;; (dom-set-attribute dom 'href (concat "AMENDED-" href))
(setcar (dom-children dom) (concat "AMENDED-" text)))
;; Process all tags normally.
(eww-tag-a dom)
;; Override the keymaps so we get called for new pages.
(when href
(put-text-property start (point)
'keymap
(if (mm-images-in-region-p start (point))
image-link-keymap
link-keymap))))))
;; Add to your minor mode and substitute this for your advice.
(add-hook 'eww-after-render-hook #'recenter-after-render)
;; Please `remove-hook` this hook when your minor mode is disabled.
;; Below would replace your call to `(eww url)` in `devdocs-browser--eww-open`
(let ((shr-external-rendering-functions
(external-rendering-functions))
(recenter-needed t)
(url (browse-url-file-url (expand-file-name "~/test.html"))))
(define-key link-keymap (kbd "RET") #'follow-link)
(define-key link-keymap (kbd "<mouse-2>") #'follow-link)
(define-key image-link-keymap (kbd "RET") #'follow-link)
(eww url)))))If you aren't familiar with cl-labels that's just a neat way to avoid polluting the global function namespace during testing. You'd obviously be referring to variables and calling functions from your devdocs-browser- name space. I did have to make one global concession for the dynamically defined recenter-needed for which you already have the buffer-local variable devdocs-browser--eww-data.
<!DOCTYPE html>
<html>
<body>
<h1>My First Heading</h1>
<p>My first paragraph.</p>
<a href="https://www.w3schools.com">This is an https link</a>
<p>
<a href="test.html">This is a file link</a>
</body>
</html>