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

Rewrite all advice functions with nadvice #147

Open
conao3 opened this issue Aug 13, 2019 · 4 comments
Open

Rewrite all advice functions with nadvice #147

conao3 opened this issue Aug 13, 2019 · 4 comments

Comments

@conao3
Copy link
Collaborator

conao3 commented Aug 13, 2019

Nowadays, advice with defadvice is not a good choice.
So we should rewrite all advice functions with nadvice.

@cireu
Copy link
Collaborator

cireu commented Aug 13, 2019

IMO, they shouldn't use advice at all, advice just an adhoc solution for modify function in other packages without touch their source code. Using specific keymap would be better.

Please have a look at #150

@cireu
Copy link
Collaborator

cireu commented Aug 13, 2019

Maybe we can get some inspiration from https://github.com/emacs-helm/helm/blob/master/helm-occur.el#L301

@cireu
Copy link
Collaborator

cireu commented Aug 13, 2019

Close in #150

@conao3 AFIAK, you can add magick word Close #xxx in the merge commit and Github will close corresponded issue automatically.

@cireu cireu closed this as completed Aug 13, 2019
@conao3
Copy link
Collaborator Author

conao3 commented Aug 13, 2019

I know, but there are still remained another defadvice don't you?

@conao3 conao3 reopened this Aug 13, 2019
@cireu cireu added 2.0.0 Will be contained in Release 2.0.0 and removed 2.0.0 Will be contained in Release 2.0.0 labels Aug 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants