From b0caf8a8f96fafb0bcc6bbad79ad35b64d741342 Mon Sep 17 00:00:00 2001 From: Nikita Bloshchanevich Date: Sat, 13 Mar 2021 10:54:41 +0100 Subject: [PATCH 01/11] Improve Advice section `helpful--skip-advice' now actually works with advice other than :around, and properly deletes the newlines added by the advice functions. All advice is now displayed in the "Advice" section, and not in the docstring. --- helpful.el | 44 +++++++++++++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/helpful.el b/helpful.el index 723746c..c586b0a 100644 --- a/helpful.el +++ b/helpful.el @@ -2303,10 +2303,19 @@ state of the current symbol." (when (helpful--advised-p helpful--sym) (helpful--insert-section-break) - (insert - (helpful--heading "Advice") - (format "This %s is advised." - (if (macrop helpful--sym) "macro" "function")))) + (insert (helpful--heading "Advice")) + (dolist (x (helpful--get-advice helpful--sym)) + (cl-destructuring-bind (combinator . advice) x + (insert (propertize (symbol-name combinator) 'face 'font-lock-builtin-face) + " " + (helpful--button + (symbol-name advice) 'helpful-describe-button + 'symbol advice + 'callable-p t) + "\n"))) + ;; We've inserted one newline too many, since the next section will insert + ;; a section break. + (delete-char -1)) (let ((can-edebug (helpful--can-edebug-p helpful--sym helpful--callable-p buf pos)) @@ -2421,16 +2430,29 @@ state of the current symbol." (when opened (kill-buffer buf)))) +(defconst helpful--advice-regexp "^\\(?:This function has \\)?\\(:[-a-z]+\\) advice: `\\(.*\\)'\\.$") + ;; TODO: this isn't sufficient for `edebug-eval-defun'. (defun helpful--skip-advice (docstring) "Remove mentions of advice from DOCSTRING." - (let* ((lines (s-lines docstring)) - (relevant-lines - (--drop-while - (or (s-starts-with-p ":around advice:" it) - (s-starts-with-p "This function has :around advice:" it)) - lines))) - (s-trim (s-join "\n" relevant-lines)))) + (with-temp-buffer + (insert docstring) + (goto-char (point-min)) + (save-match-data + (while (looking-at helpful--advice-regexp) + (delete-region (match-beginning 0) (1+ (match-end 0))))) + (buffer-substring-no-properties (point-min) (point-max)))) + +(defun helpful--extract-advice (docstring) + (save-match-data + (cl-loop with lines = (s-lines docstring) for line = (car lines) + while (and lines (string-match helpful--advice-regexp line)) + collect (cons (intern (match-string-no-properties 1 line)) + (intern (match-string-no-properties 2 line))) + do (pop lines)))) + +(defun helpful--get-advice (sym) + (helpful--extract-advice (let ((text-quoting-style 'grave)) (documentation sym t)))) (defun helpful--format-argument (arg) "Format ARG (a symbol) according to Emacs help conventions." From 6d768c65ba3ef65e88ac126ae76a664930719090 Mon Sep 17 00:00:00 2001 From: Nikita Bloshchanevich Date: Sat, 13 Mar 2021 10:57:48 +0100 Subject: [PATCH 02/11] Move "Advice" section after "Documentation" "Advice" is a relatively important section, so move it closer to "Documentation", which used to display most advice. --- helpful.el | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/helpful.el b/helpful.el index c586b0a..c8b20d5 100644 --- a/helpful.el +++ b/helpful.el @@ -2257,6 +2257,22 @@ state of the current symbol." (insert "\n\n") (insert (helpful--make-manual-button helpful--sym))))) + (when (helpful--advised-p helpful--sym) + (helpful--insert-section-break) + (insert (helpful--heading "Advice")) + (dolist (x (helpful--get-advice helpful--sym)) + (cl-destructuring-bind (combinator . advice) x + (insert (propertize (symbol-name combinator) 'face 'font-lock-builtin-face) + " " + (helpful--button + (symbol-name advice) 'helpful-describe-button + 'symbol advice + 'callable-p t) + "\n"))) + ;; We've inserted one newline too many, since the next section will insert + ;; a section break. + (delete-char -1)) + ;; Show keybindings. ;; TODO: allow users to conveniently add and remove keybindings. (when (commandp helpful--sym) @@ -2301,22 +2317,6 @@ state of the current symbol." " " (helpful--make-callees-button helpful--sym source))) - (when (helpful--advised-p helpful--sym) - (helpful--insert-section-break) - (insert (helpful--heading "Advice")) - (dolist (x (helpful--get-advice helpful--sym)) - (cl-destructuring-bind (combinator . advice) x - (insert (propertize (symbol-name combinator) 'face 'font-lock-builtin-face) - " " - (helpful--button - (symbol-name advice) 'helpful-describe-button - 'symbol advice - 'callable-p t) - "\n"))) - ;; We've inserted one newline too many, since the next section will insert - ;; a section break. - (delete-char -1)) - (let ((can-edebug (helpful--can-edebug-p helpful--sym helpful--callable-p buf pos)) (can-trace From 478ef0dd4d68f65a15a187683a873c3f25333ec4 Mon Sep 17 00:00:00 2001 From: Nikita Bloshchanevich Date: Sat, 13 Mar 2021 11:46:16 +0100 Subject: [PATCH 03/11] Add buttons to remove advice In the "Advice" section, a clickable X is displayed that will remove the given piece of advice. --- helpful.el | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/helpful.el b/helpful.el index c8b20d5..e520498 100644 --- a/helpful.el +++ b/helpful.el @@ -1115,6 +1115,16 @@ unescaping too." 'follow-link t 'help-echo "Follow this link") +(defun helpful--remove-advice (button) + "Remove advice for BUTTON." + (advice-remove (button-get button 'symbol) + (button-get button 'advice)) + (helpful-update)) + +(define-button-type 'helpful-remove-advice-button + 'action #'helpful--remove-advice + 'help-echo "Remove advice") + (defun helpful--propertize-links (docstring) "Convert URL links in docstrings to buttons." (replace-regexp-in-string @@ -2262,7 +2272,11 @@ state of the current symbol." (insert (helpful--heading "Advice")) (dolist (x (helpful--get-advice helpful--sym)) (cl-destructuring-bind (combinator . advice) x - (insert (propertize (symbol-name combinator) 'face 'font-lock-builtin-face) + (insert (helpful--button "X" 'helpful-remove-advice-button + 'symbol helpful--sym + 'advice advice) + " " + (propertize (symbol-name combinator) 'face 'font-lock-builtin-face) " " (helpful--button (symbol-name advice) 'helpful-describe-button @@ -2434,7 +2448,8 @@ state of the current symbol." ;; TODO: this isn't sufficient for `edebug-eval-defun'. (defun helpful--skip-advice (docstring) - "Remove mentions of advice from DOCSTRING." + "Remove mentions of advice from DOCSTRING. +The resulting DOCSTRING might start with a blank newline." (with-temp-buffer (insert docstring) (goto-char (point-min)) From 99ed21a94b2d3cf798ffb594b2204246142dcf7b Mon Sep 17 00:00:00 2001 From: Nikita Bloshchanevich Date: Sat, 13 Mar 2021 11:57:21 +0100 Subject: [PATCH 04/11] List advice in chronological order `advice-add' and `defadvice' add `advice' to the front of the docstring, so `helpful--extract-advice' ends up displaying them in reverse-chronological order. --- helpful.el | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/helpful.el b/helpful.el index e520498..0a9743c 100644 --- a/helpful.el +++ b/helpful.el @@ -2459,12 +2459,15 @@ The resulting DOCSTRING might start with a blank newline." (buffer-substring-no-properties (point-min) (point-max)))) (defun helpful--extract-advice (docstring) - (save-match-data - (cl-loop with lines = (s-lines docstring) for line = (car lines) - while (and lines (string-match helpful--advice-regexp line)) - collect (cons (intern (match-string-no-properties 1 line)) - (intern (match-string-no-properties 2 line))) - do (pop lines)))) + (let ((lines (s-lines docstring)) + line result) + (save-match-data + (while (and lines (string-match helpful--advice-regexp + (setq line (pop lines)))) + (push (cons (intern (match-string-no-properties 1 line)) + (intern (match-string-no-properties 2 line))) + result)) + result))) (defun helpful--get-advice (sym) (helpful--extract-advice (let ((text-quoting-style 'grave)) (documentation sym t)))) From 5770bdba5a9291b14c2bb349be4b1e6d11336cd9 Mon Sep 17 00:00:00 2001 From: Nikita Bloshchanevich Date: Sat, 13 Mar 2021 11:59:58 +0100 Subject: [PATCH 05/11] Add docstrings --- helpful.el | 2 ++ 1 file changed, 2 insertions(+) diff --git a/helpful.el b/helpful.el index 0a9743c..640bad0 100644 --- a/helpful.el +++ b/helpful.el @@ -2459,6 +2459,7 @@ The resulting DOCSTRING might start with a blank newline." (buffer-substring-no-properties (point-min) (point-max)))) (defun helpful--extract-advice (docstring) + "Extract `advice' from DOCSTRING." (let ((lines (s-lines docstring)) line result) (save-match-data @@ -2470,6 +2471,7 @@ The resulting DOCSTRING might start with a blank newline." result))) (defun helpful--get-advice (sym) + "Extract `advice' from SYM." (helpful--extract-advice (let ((text-quoting-style 'grave)) (documentation sym t)))) (defun helpful--format-argument (arg) From c0f5e075ea1d7c3a04bf2b37c87b26a973efb76e Mon Sep 17 00:00:00 2001 From: Nikita Bloshchanevich Date: Sat, 13 Mar 2021 15:59:48 +0100 Subject: [PATCH 06/11] Add a "Remove all advice" button and command `helpful-remove-all-advice' removes all advice for the current helpful button. There is now a button in the Debugging section that removes all advice, which is enabled only if the function is advised. --- helpful.el | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/helpful.el b/helpful.el index 640bad0..142ea18 100644 --- a/helpful.el +++ b/helpful.el @@ -1125,6 +1125,30 @@ unescaping too." 'action #'helpful--remove-advice 'help-echo "Remove advice") +(defun helpful--remove-all-advice-1 (sym) + "Remove all advice from SYM." + (advice-mapc (lambda (advice x) (advice-remove sym advice)) sym) + (helpful-update)) + +(defun helpful--remove-all-advice (button) + "Remove all advice for BUTTON." + (helpful--remove-all-advice-1 (button-get button 'symbol))) + +(define-button-type 'helpful-remove-all-advice-button + 'action #'helpful--remove-all-advice + 'help-echo "Remove all advice") + +(defun helpful-remove-all-advice () + "Remove all advice for the current helpful symbol." + (interactive) + (unless (derived-mode-p #'helpful-mode) + (user-error "Must be in a *helpful* buffer")) + (unless helpful--callable-p + (user-error "Cannot unadvise a variable")) + (unless (helpful--advised-p helpful--sym) + (user-error "Function not advised")) + (helpful--remove-all-advice-1 helpful--sym)) + (defun helpful--propertize-links (docstring) "Convert URL links in docstrings to buttons." (replace-regexp-in-string @@ -2169,7 +2193,8 @@ state of the current symbol." (references (helpful--calculate-references helpful--sym helpful--callable-p source-path)) - (aliases (helpful--aliases helpful--sym helpful--callable-p))) + (aliases (helpful--aliases helpful--sym helpful--callable-p)) + (advised? (helpful--advised-p helpful--sym))) (erase-buffer) @@ -2267,7 +2292,7 @@ state of the current symbol." (insert "\n\n") (insert (helpful--make-manual-button helpful--sym))))) - (when (helpful--advised-p helpful--sym) + (when advised? (helpful--insert-section-break) (insert (helpful--heading "Advice")) (dolist (x (helpful--get-advice helpful--sym)) @@ -2368,7 +2393,11 @@ state of the current symbol." (when can-forget (when can-disassemble (insert " ")) - (insert (helpful--make-forget-button helpful--sym helpful--callable-p)))) + (insert (helpful--make-forget-button helpful--sym helpful--callable-p))) + (when advised? + (when can-forget (insert " ")) + (insert (helpful--button "Remove all advice" 'helpful-remove-all-advice-button + 'symbol helpful--sym)))) (when aliases (helpful--insert-section-break) From aa4b200e62781f6c43eff5a6a03061845a22b1cc Mon Sep 17 00:00:00 2001 From: Nikita Bloshchanevich Date: Sun, 14 Mar 2021 13:31:13 +0100 Subject: [PATCH 07/11] Fix `helpful--skip-advice' - Make the dot optional, supporting Emacs pre Emacs-27 - Remove the newline after advice, fixing the CI - Only skip advice if the function is advised (since `helpful--skip-advice' isn't 100% strict) --- helpful.el | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/helpful.el b/helpful.el index 142ea18..6ca65b2 100644 --- a/helpful.el +++ b/helpful.el @@ -2473,18 +2473,23 @@ state of the current symbol." (when opened (kill-buffer buf)))) -(defconst helpful--advice-regexp "^\\(?:This function has \\)?\\(:[-a-z]+\\) advice: `\\(.*\\)'\\.$") +(defconst helpful--advice-regexp + "^\\(?:This function has \\)?\\(:[-a-z]+\\) advice: `\\(.*\\)'\\.?$" + "Regexp matching advice lines. +Match group 1 is the combinator, with colon, and match group 2 is +the advice.") ;; TODO: this isn't sufficient for `edebug-eval-defun'. (defun helpful--skip-advice (docstring) - "Remove mentions of advice from DOCSTRING. -The resulting DOCSTRING might start with a blank newline." + "Remove mentions of advice from DOCSTRING." (with-temp-buffer (insert docstring) (goto-char (point-min)) (save-match-data (while (looking-at helpful--advice-regexp) (delete-region (match-beginning 0) (1+ (match-end 0))))) + (when (eq (char-after) ?\n) + (delete-char 1)) (buffer-substring-no-properties (point-min) (point-max)))) (defun helpful--extract-advice (docstring) @@ -2600,10 +2605,7 @@ escapes that are used by `substitute-command-keys'." (setq docstring (documentation sym t)) (-when-let (docstring-with-usage (help-split-fundoc docstring sym)) (setq docstring (cdr docstring-with-usage)) - (when docstring - ;; Advice mutates the docstring, see - ;; `advice--make-docstring'. Undo that. - ;; TODO: Only do this if the function is advised. + (when (helpful--advised-p sym) (setq docstring (helpful--skip-advice docstring))))) (setq docstring (documentation-property sym 'variable-documentation t))) From 957978aa54ef6909691131fa993bf42656375db7 Mon Sep 17 00:00:00 2001 From: Nikita Bloshchanevich Date: Sun, 14 Mar 2021 14:11:32 +0100 Subject: [PATCH 08/11] Fix spacing before remove all advice button --- helpful.el | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/helpful.el b/helpful.el index 6ca65b2..d82f73d 100644 --- a/helpful.el +++ b/helpful.el @@ -2384,7 +2384,7 @@ state of the current symbol." (when (and (or can-edebug can-trace) - (or can-disassemble can-forget)) + (or can-disassemble can-forget advised?)) (insert "\n")) (when can-disassemble @@ -2395,7 +2395,8 @@ state of the current symbol." (insert " ")) (insert (helpful--make-forget-button helpful--sym helpful--callable-p))) (when advised? - (when can-forget (insert " ")) + (when (or can-forget can-disassemble) + (insert " ")) (insert (helpful--button "Remove all advice" 'helpful-remove-all-advice-button 'symbol helpful--sym)))) From d89c5dc69772f2e42b9b8740f3f459d6200adfc5 Mon Sep 17 00:00:00 2001 From: Nikita Bloshchanevich Date: Sun, 14 Mar 2021 14:11:53 +0100 Subject: [PATCH 09/11] Shorten long line --- helpful.el | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/helpful.el b/helpful.el index d82f73d..ffea5f8 100644 --- a/helpful.el +++ b/helpful.el @@ -2507,7 +2507,8 @@ the advice.") (defun helpful--get-advice (sym) "Extract `advice' from SYM." - (helpful--extract-advice (let ((text-quoting-style 'grave)) (documentation sym t)))) + (helpful--extract-advice (let ((text-quoting-style 'grave)) + (documentation sym t)))) (defun helpful--format-argument (arg) "Format ARG (a symbol) according to Emacs help conventions." From ca6a3288f74ec9ba8e955da15d37e26501124227 Mon Sep 17 00:00:00 2001 From: Nikita Bloshchanevich Date: Mon, 15 Mar 2021 10:21:17 +0100 Subject: [PATCH 10/11] More robust advice docstring handling Sometimes, there can be newlines between advice in docstrings. Handle those cases. --- helpful.el | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/helpful.el b/helpful.el index ffea5f8..25e17f2 100644 --- a/helpful.el +++ b/helpful.el @@ -2475,7 +2475,7 @@ state of the current symbol." (kill-buffer buf)))) (defconst helpful--advice-regexp - "^\\(?:This function has \\)?\\(:[-a-z]+\\) advice: `\\(.*\\)'\\.?$" + "^\\(?:This function has \\)?\\(:[-a-z]+\\) advice: `\\(.*\\)'\\.?\n\n?" "Regexp matching advice lines. Match group 1 is the combinator, with colon, and match group 2 is the advice.") @@ -2488,22 +2488,22 @@ the advice.") (goto-char (point-min)) (save-match-data (while (looking-at helpful--advice-regexp) - (delete-region (match-beginning 0) (1+ (match-end 0))))) - (when (eq (char-after) ?\n) - (delete-char 1)) + (delete-region (match-beginning 0) (match-end 0)))) (buffer-substring-no-properties (point-min) (point-max)))) (defun helpful--extract-advice (docstring) "Extract `advice' from DOCSTRING." - (let ((lines (s-lines docstring)) - line result) + (with-temp-buffer + (insert docstring) + (goto-char (point-min)) (save-match-data - (while (and lines (string-match helpful--advice-regexp - (setq line (pop lines)))) - (push (cons (intern (match-string-no-properties 1 line)) - (intern (match-string-no-properties 2 line))) - result)) - result))) + (let (result) + (while (looking-at helpful--advice-regexp) + (push (cons (intern (match-string-no-properties 1)) + (intern (match-string-no-properties 2))) + result) + (goto-char (match-end 0))) + result)))) (defun helpful--get-advice (sym) "Extract `advice' from SYM." From 0c732012913f431bd87ede8f0d5069f2700ad82e Mon Sep 17 00:00:00 2001 From: Nikita Bloshchanevich Date: Mon, 15 Mar 2021 10:25:59 +0100 Subject: [PATCH 11/11] Use `read' for advice Advice names may contain escapes, which are handled with `read'. --- helpful.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/helpful.el b/helpful.el index 25e17f2..01456f8 100644 --- a/helpful.el +++ b/helpful.el @@ -2304,7 +2304,7 @@ state of the current symbol." (propertize (symbol-name combinator) 'face 'font-lock-builtin-face) " " (helpful--button - (symbol-name advice) 'helpful-describe-button + (helpful--format-symbol advice) 'helpful-describe-button 'symbol advice 'callable-p t) "\n"))) @@ -2500,7 +2500,7 @@ the advice.") (let (result) (while (looking-at helpful--advice-regexp) (push (cons (intern (match-string-no-properties 1)) - (intern (match-string-no-properties 2))) + (read (match-string-no-properties 2))) result) (goto-char (match-end 0))) result))))