Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Advice section #265

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

nbfalcon
Copy link
Contributor

  • helpful--skip-advice now works for non :around advice. This means that the
    docstring no longer displays any advice.
  • The Advice section no longer just says "This function is advised", but now
    lists the advice, one per line, with the format <clickable X to remove> :<before/after/...> <advice symbol>:
    X :before +whatever
    X :after +whatever

`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.
"Advice" is a relatively important section, so move it closer to
"Documentation", which used to display most advice.
In the "Advice" section, a clickable X is displayed that will remove the
given piece of advice.
`advice-add' and `defadvice' add `advice' to the front of the docstring, so
`helpful--extract-advice' ends up displaying them in reverse-chronological
order.
@nbfalcon nbfalcon changed the title Add buttons to remove advice Improve Advice section Mar 13, 2021
@nbfalcon
Copy link
Contributor Author

Here is how this looks now:
20210313-120043

The buffer in the middle shows the code used to create the function with advice. defadvice! is a doom-emacs advice definition functions, and it is basically defun + advice-add (i.e. using nadvice), so this works with both kinds of advice. A limitation of the current implementation is that defadvice (the old one) advice does not display the actual advice function, but ad-Advice-. IMHO this is better addressed upstream, but I can implement better display if you wish.

@nbfalcon
Copy link
Contributor Author

For comparison, here is how this looks in master (helpful--skip-advice` doesn't work properly):
20210313-120544

`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.
- 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)
@nbfalcon nbfalcon force-pushed the feature/advice-section-list-advice branch from eff60eb to d89c5dc Compare March 14, 2021 13:47
Sometimes, there can be newlines between advice in docstrings. Handle
those cases.
Advice names may contain escapes, which are handled with `read'.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant