Re: Drop defadvice from Org
Hi Ihor, Ihor Radchenko writes: > The change in (eval ...) call inside org-diary-sexp-entry broke sexp > timestamps. See orgmode.org/list/875ynnojvf.fsf@localhost Fixed, thanks! -- Bastien
Re: Drop defadvice from Org
Bastien Guerry writes: > Stefan Monnier writes: > >> The patch below gets rid of the old `defadvice`, replacing it with >> `advice-add`. > > Applied in the main branch as 6d73cd34a, thanks a lot! The change in (eval ...) call inside org-diary-sexp-entry broke sexp timestamps. See orgmode.org/list/875ynnojvf.fsf@localhost I think that calendar-related evals should be reverted to use dynamic scope. AFAIU, diary staff is relying on dynamic scope and cannot be used with lexical. Best, Ihor
Re: Drop defadvice from Org
> i was ok with the scold for a long time about (` thing but my reactin > time slowed significantly and that was trivial-er. The (` transition was not handled ideally, to be honest: we declared them obsolete very early but only started emitting warnings much later, so the transition period have seemed short for those who only learned about it when the warning appeared. > 2040 is when i will begin figuring out or finding some long lost > convert thing, and then decide to do something about my carefully > self-cargo-culted advice. fortunately, debian on the trailing edge > will give me a bit more. Also, note that ever since `nadvice.el` appeared, `advice.el` was "reimplemented" as a layer on top of `nadvice.el`, so when it finally gets removed from Emacs, I'd expect that `advice.el` could be added to GNU ELPA for those who want to keep using it (typically because of some old and unmaintained package). [ The same might happen with `cl.el`. ] Stefan
Re: Drop defadvice from Org
On 3/31/22, Stefan Monnier wrote: > I definitely hope it will be gone before 2040, but it hasn't even been > declared officially obsolete yet (not even in `master`), so I think you > should be good at least until 2030. thanks. i was ok with the scold for a long time about (` thing but my reactin time slowed significantly and that was trivial-er. 2040 is when i will begin figuring out or finding some long lost convert thing, and then decide to do something about my carefully self-cargo-culted advice. fortunately, debian on the trailing edge will give me a bit more. still trying to get my scripts to understand the new branch names.
Re: Drop defadvice from Org
Hi Stefan, Stefan Monnier writes: > The patch below gets rid of the old `defadvice`, replacing it with > `advice-add`. Applied in the main branch as 6d73cd34a, thanks a lot! -- Bastien
Re: Drop defadvice from Org
> thank you. just an idle question. is it common/desirable for built > in packages to use advice instead of hooks and such? Desirable? no. Common? kinda, yes, sadly. It's usually good to look at the existing advice as "requests for hooks". I haven't spent the energy to look at them this way, tho. IOW patches welcome. > also, merely as a plea from a user, I hope defadvice will stick around > for all that user and non-built-in and abandoned code. I definitely hope it will be gone before 2040, but it hasn't even been declared officially obsolete yet (not even in `master`), so I think you should be good at least until 2030. Stefan
Re: Drop defadvice from Org
thank you. just an idle question. is it common/desirable for built in packages to use advice instead of hooks and such? also, merely as a plea from a user, i hope defadvice will stick around for all that user and non-built-in and abandoned code. On 3/31/22, Stefan Monnier wrote: > The patch below gets rid of the old `defadvice`, replacing it with > `advice-add`. > It also includes some FIXMEs about things I found along the way which > look suspicious (they're not directly related to the patch, tho, nor > are they affected by it AFAICT). > > > Stefan > > > 2022-03-31 Stefan Monnier > > Replace all uses of the old `defadvice` with the new `advice-add`. > Along the way, remove some redundant `:group` args > (redundant because they specify the same group as would be used by > default anyway) and make a few other simplifications. > Also don't bother putting `advice-add` within an eval-after-load > since the advice machinery already takes care of handling it. > > * lisp/org.el (org-run-like-in-org-mode): Strength reduce `eval` > to `cl-progv`. > (org--check-org-structure-template-alist): Strength reduce `eval` > to `symbol-value`. > (org-map-entries, org-eval-in-calendar, org-diary-sexp-entry): > Make sure we use the new lexically scoped dialect. > (org--math-always-on): New function, extracted from advice. > (org-cdlatex-mode): Use it with `advice-add`. > (org-self-insert-command): Simplify `and`+`listp` into `consp`. > (org-submit-bug-report): > Make sure we use the new lexically scoped dialect. > > * lisp/org-protocol.el (org-protocol-convert-query-to-plist): > Use `cl-mapcan`. > (org--protocol-detect-protocol-server): New function, extracted > from advice. > (server-visit-files): Use it with `advice-add`. > > * lisp/org-mouse.el (org--mouse-dnd-insert-text): New function, > extracted > from advice. > (dnd-insert-text): Use it with `advice-add`. > (org--mouse-dnd-open-file): New function, extracted from advice. > (dnd-open-file): Use it with `advice-add`. > (org--mouse-open-at-point): New function, extracted from advice. > (org-mode-hook): Advise `org-open-at-point` with `advice-add`. > > * lisp/org-ctags.el (org--ctags-load-tag-list): New function, extracted > from advice. > (visit-tags-table): Use it with `advice-add`. > (org--ctags-set-org-mark-before-finding-tag): New function, extracted > from advice. > (xref-find-definitions): Use it with `advice-add`. > > * lisp/org-compat.el (org-bookmark-jump-unhide): Accept (unused) args. > (save-place-find-file-hook): Use `advice-add`. > (org--ecb-show-context): New function, extracted from advice. > (ecb-method-clicked): Use it with `advice-add`. > (org-mark-jump-unhide): Accept (unused) args. > (pop-to-mark-command, exchange-point-and-mark, pop-global-mark): > Use `advice-add`. > > > diff --git a/lisp/org-compat.el b/lisp/org-compat.el > index 38d330de6d..f768a8233b 100644 > --- a/lisp/org-compat.el > +++ b/lisp/org-compat.el > @@ -901,7 +901,6 @@ attention to case differences." > (defcustom org-imenu-depth 2 >"The maximum level for Imenu access to Org headlines. > This also applied for speedbar access." > - :group 'org-imenu-and-speedbar >:type 'integer) > > Imenu > @@ -1114,7 +1113,7 @@ ELEMENT is the element at point." > > Bookmark > > -(defun org-bookmark-jump-unhide () > +(defun org-bookmark-jump-unhide ( _) >"Unhide the current position, to show the bookmark location." >(and (derived-mode-p 'org-mode) > (or (org-invisible-p) > @@ -1123,7 +1122,7 @@ ELEMENT is the element at point." > (org-show-context 'bookmark-jump))) > > ;; Make `bookmark-jump' shows the jump location if it was hidden. > -(add-hook 'bookmark-after-jump-hook 'org-bookmark-jump-unhide) > +(add-hook 'bookmark-after-jump-hook #'org-bookmark-jump-unhide) > > Calendar > > @@ -1176,42 +1175,29 @@ key." > Saveplace > > ;; Make sure saveplace shows the location if it was hidden > -(eval-after-load 'saveplace > - '(defadvice save-place-find-file-hook (after org-make-visible activate) > - "Make the position visible." > - (org-bookmark-jump-unhide))) > +(advice-add 'save-place-find-file-hook :after #'org-bookmark-jump-unhide) > > Ecb > > ;; Make sure ecb shows the location if it was hidden > -(eval-after-load 'ecb > - '(defadvice ecb-method-clicked (after esf/org-show-context activate) > - "Make hierarchy visible when jumping into location from ECB tree > buffer." > - (when (derived-mode-p 'org-mode) > - (org-show-context > +(advice-add 'ecb-method-clicked :after #'org--ecb-show-context) > +(defun org--ecb-show-context ( _) > + "Make hierarchy visible when jumping into location from ECB tree > buffer." > + (when (derived-mode-p 'org-mode) > +(org-show-context))) > > Simple > > -(defun org-mark-jump-unhide () >
Drop defadvice from Org
The patch below gets rid of the old `defadvice`, replacing it with `advice-add`. It also includes some FIXMEs about things I found along the way which look suspicious (they're not directly related to the patch, tho, nor are they affected by it AFAICT). Stefan 2022-03-31 Stefan Monnier Replace all uses of the old `defadvice` with the new `advice-add`. Along the way, remove some redundant `:group` args (redundant because they specify the same group as would be used by default anyway) and make a few other simplifications. Also don't bother putting `advice-add` within an eval-after-load since the advice machinery already takes care of handling it. * lisp/org.el (org-run-like-in-org-mode): Strength reduce `eval` to `cl-progv`. (org--check-org-structure-template-alist): Strength reduce `eval` to `symbol-value`. (org-map-entries, org-eval-in-calendar, org-diary-sexp-entry): Make sure we use the new lexically scoped dialect. (org--math-always-on): New function, extracted from advice. (org-cdlatex-mode): Use it with `advice-add`. (org-self-insert-command): Simplify `and`+`listp` into `consp`. (org-submit-bug-report): Make sure we use the new lexically scoped dialect. * lisp/org-protocol.el (org-protocol-convert-query-to-plist): Use `cl-mapcan`. (org--protocol-detect-protocol-server): New function, extracted from advice. (server-visit-files): Use it with `advice-add`. * lisp/org-mouse.el (org--mouse-dnd-insert-text): New function, extracted from advice. (dnd-insert-text): Use it with `advice-add`. (org--mouse-dnd-open-file): New function, extracted from advice. (dnd-open-file): Use it with `advice-add`. (org--mouse-open-at-point): New function, extracted from advice. (org-mode-hook): Advise `org-open-at-point` with `advice-add`. * lisp/org-ctags.el (org--ctags-load-tag-list): New function, extracted from advice. (visit-tags-table): Use it with `advice-add`. (org--ctags-set-org-mark-before-finding-tag): New function, extracted from advice. (xref-find-definitions): Use it with `advice-add`. * lisp/org-compat.el (org-bookmark-jump-unhide): Accept (unused) args. (save-place-find-file-hook): Use `advice-add`. (org--ecb-show-context): New function, extracted from advice. (ecb-method-clicked): Use it with `advice-add`. (org-mark-jump-unhide): Accept (unused) args. (pop-to-mark-command, exchange-point-and-mark, pop-global-mark): Use `advice-add`. diff --git a/lisp/org-compat.el b/lisp/org-compat.el index 38d330de6d..f768a8233b 100644 --- a/lisp/org-compat.el +++ b/lisp/org-compat.el @@ -901,7 +901,6 @@ attention to case differences." (defcustom org-imenu-depth 2 "The maximum level for Imenu access to Org headlines. This also applied for speedbar access." - :group 'org-imenu-and-speedbar :type 'integer) Imenu @@ -1114,7 +1113,7 @@ ELEMENT is the element at point." Bookmark -(defun org-bookmark-jump-unhide () +(defun org-bookmark-jump-unhide ( _) "Unhide the current position, to show the bookmark location." (and (derived-mode-p 'org-mode) (or (org-invisible-p) @@ -1123,7 +1122,7 @@ ELEMENT is the element at point." (org-show-context 'bookmark-jump))) ;; Make `bookmark-jump' shows the jump location if it was hidden. -(add-hook 'bookmark-after-jump-hook 'org-bookmark-jump-unhide) +(add-hook 'bookmark-after-jump-hook #'org-bookmark-jump-unhide) Calendar @@ -1176,42 +1175,29 @@ key." Saveplace ;; Make sure saveplace shows the location if it was hidden -(eval-after-load 'saveplace - '(defadvice save-place-find-file-hook (after org-make-visible activate) - "Make the position visible." - (org-bookmark-jump-unhide))) +(advice-add 'save-place-find-file-hook :after #'org-bookmark-jump-unhide) Ecb ;; Make sure ecb shows the location if it was hidden -(eval-after-load 'ecb - '(defadvice ecb-method-clicked (after esf/org-show-context activate) - "Make hierarchy visible when jumping into location from ECB tree buffer." - (when (derived-mode-p 'org-mode) - (org-show-context +(advice-add 'ecb-method-clicked :after #'org--ecb-show-context) +(defun org--ecb-show-context ( _) + "Make hierarchy visible when jumping into location from ECB tree buffer." + (when (derived-mode-p 'org-mode) +(org-show-context))) Simple -(defun org-mark-jump-unhide () +(defun org-mark-jump-unhide ( _) "Make the point visible with `org-show-context' after jumping to the mark." (when (and (derived-mode-p 'org-mode) (org-invisible-p)) (org-show-context 'mark-goto))) -(eval-after-load 'simple - '(defadvice pop-to-mark-command (after org-make-visible activate) - "Make the point visible with `org-show-context'." - (org-mark-jump-unhide))) +(advice-add 'pop-to-mark-command :after #'org-mark-jump-unhide) -(eval-after-load
