Drop defadvice from Org

Drop defadvice from Org

[Stefan Monnier]

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  <monnier@iro.umontreal.ca>

    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 (&rest _)
   "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 (&rest _)
+  "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 (&rest _)
   "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 'simple
-  '(defadvice exchange-point-and-mark (after org-make-visible activate)
-     "Make the point visible with `org-show-context'."
-     (org-mark-jump-unhide)))
-
-(eval-after-load 'simple
-  '(defadvice pop-global-mark (after org-make-visible activate)
-     "Make the point visible with `org-show-context'."
-     (org-mark-jump-unhide)))
+(advice-add 'exchange-point-and-mark :after #'org-mark-jump-unhide)
+(advice-add 'pop-global-mark :after #'org-mark-jump-unhide)
 
 ;;;; Session
 
diff --git a/lisp/org-ctags.el b/lisp/org-ctags.el
index 6fc97ca399..59a08d0b54 100644
--- a/lisp/org-ctags.el
+++ b/lisp/org-ctags.el
@@ -157,7 +157,6 @@ See the ctags documentation for more information.")
 (defcustom org-ctags-path-to-ctags
   (if (executable-find "ctags-exuberant") "ctags-exuberant" "ctags")
   "Name of the ctags executable file."
-  :group 'org-ctags
   :version "24.1"
   :type 'file)
 
@@ -166,7 +165,6 @@ See the ctags documentation for more information.")
     org-ctags-ask-rebuild-tags-file-then-find-tag
     org-ctags-ask-append-topic)
   "List of functions to be prepended to ORG-OPEN-LINK-FUNCTIONS by ORG-CTAGS."
-  :group 'org-ctags
   :version "24.1"
   :type 'hook
   :options '(org-ctags-find-tag
@@ -188,7 +186,6 @@ Created as a local variable in each buffer.")
   "Text to insert when creating a new org file via opening a hyperlink.
 The following patterns are replaced in the string:
     `%t' - replaced with the capitalized title of the hyperlink"
-  :group 'org-ctags
   :version "24.1"
   :type 'string)
 
@@ -207,7 +204,8 @@ The following patterns are replaced in the string:
                   (visit-tags-table tags-filename))))))
 
 
-(defadvice visit-tags-table (after org-ctags-load-tag-list activate compile)
+(advice-add 'visit-tags-table :after #'org--ctags-load-tag-list)
+(defun org--ctags-load-tag-list (&rest _)
   (when (and org-ctags-enabled-p tags-file-name)
     (setq-local org-ctags-tag-list
 		(org-ctags-all-tags-in-current-tags-table))))
@@ -295,8 +293,9 @@ The new topic will be titled NAME (or TITLE if supplied)."
 ;;;; Misc interoperability with etags system =================================
 
 
-(defadvice xref-find-definitions
-    (before org-ctags-set-org-mark-before-finding-tag activate compile)
+(advice-add 'xref-find-definitions :before
+            #'org--ctags-set-org-mark-before-finding-tag)
+(defun org--ctags-set-org-mark-before-finding-tag (&rest _)
   "Before trying to find a tag, save our current position on org mark ring."
   (save-excursion
     (when (and (derived-mode-p 'org-mode) org-ctags-enabled-p)
diff --git a/lisp/org-mouse.el b/lisp/org-mouse.el
index 20c20acc32..2d8136b752 100644
--- a/lisp/org-mouse.el
+++ b/lisp/org-mouse.el
@@ -580,15 +580,17 @@ This means, between the beginning of line and the point."
   (insert text)
   (beginning-of-line))
 
-(defadvice dnd-insert-text (around org-mouse-dnd-insert-text activate)
+(advice-add 'dnd-insert-text :around #'org--mouse-dnd-insert-text)
+(defun org--mouse-dnd-insert-text (orig-fun window action text &rest args)
   (if (derived-mode-p 'org-mode)
       (org-mouse-insert-item text)
-    ad-do-it))
+    (apply orig-fun window action text args)))
 
-(defadvice dnd-open-file (around org-mouse-dnd-open-file activate)
+(advice-add 'dnd-open-file :around #'org--mouse-dnd-open-file)
+(defun org--mouse-dnd-open-file (orig-fun uri &rest args)
   (if (derived-mode-p 'org-mode)
       (org-mouse-insert-item uri)
-    ad-do-it))
+    (apply orig-fun uri args)))
 
 (defun org-mouse-match-closure (function)
   (let ((match (match-data t)))
@@ -894,15 +896,17 @@ This means, between the beginning of line and the point."
                   (1 `(face nil keymap ,org-mouse-map mouse-face highlight) prepend)))
                t))
 
-            (defadvice org-open-at-point (around org-mouse-open-at-point activate)
-              (let ((context (org-context)))
-                (cond
-                 ((assq :headline-stars context) (org-cycle))
-                 ((assq :checkbox context) (org-toggle-checkbox))
-                 ((assq :item-bullet context)
-                  (let ((org-cycle-include-plain-lists t)) (org-cycle)))
-                 ((org-footnote-at-reference-p) nil)
-                 (t ad-do-it))))))
+            (advice-add 'org-open-at-point :around #'org--mouse-open-at-point)))
+
+(defun org--mouse-open-at-point (orig-fun &rest args)
+  (let ((context (org-context)))
+    (cond
+     ((assq :headline-stars context) (org-cycle))
+     ((assq :checkbox context) (org-toggle-checkbox))
+     ((assq :item-bullet context)
+      (let ((org-cycle-include-plain-lists t)) (org-cycle)))
+     ((org-footnote-at-reference-p) nil)
+     (t (apply orig-fun args)))))
 
 (defun org-mouse-move-tree-start (_event)
   (interactive "e")
diff --git a/lisp/org-persist.el b/lisp/org-persist.el
index 3b6a2d330a..dc14af817e 100644
--- a/lisp/org-persist.el
+++ b/lisp/org-persist.el
@@ -179,7 +179,6 @@
                                        cache-dir))
                                    "org-persist/"))
   "Directory where the data is stored."
-  :group 'org-persist
   :type 'directory)
 
 (defcustom org-persist-remote-files 100
@@ -931,8 +930,8 @@ Also, remove containers associated with non-existing files."
       (message "Missing write access rights to org-persist-directory: %S"
                org-persist-directory)
     (add-hook 'kill-emacs-hook #'org-persist-write-all)
-    ;; `org-persist-gc' should run before `org-persist-write-all'.  So we are adding the
-    ;; hook after `org-persist-write-all'.
+    ;; `org-persist-gc' should run before `org-persist-write-all'.
+    ;; So we are adding the hook after `org-persist-write-all'.
     (add-hook 'kill-emacs-hook #'org-persist-gc)))
 
 (add-hook 'after-init-hook #'org-persist-load-all)
diff --git a/lisp/org-protocol.el b/lisp/org-protocol.el
index 7c4de03bc2..1969f51fe3 100644
--- a/lisp/org-protocol.el
+++ b/lisp/org-protocol.el
@@ -174,7 +174,6 @@ The filenames passed on the command line are passed to the emacs-server in
 reverse order.  Set to t (default) to re-reverse the list, i.e. use the
 sequence on the command line.  If nil, the sequence of the filenames is
 unchanged."
-  :group 'org-protocol
   :type 'boolean)
 
 (defcustom org-protocol-project-alist nil
@@ -233,7 +232,6 @@ Example:
 Consider using the interactive functions `org-protocol-create'
 and `org-protocol-create-for-org' to help you filling this
 variable with valid contents."
-  :group 'org-protocol
   :type 'alist)
 
 (defcustom org-protocol-protocol-alist nil
@@ -284,20 +282,17 @@ Here is an example:
         (\"your-protocol\"
          :protocol \"your-protocol\"
          :function your-protocol-handler-function)))"
-  :group 'org-protocol
   :type '(alist))
 
 (defcustom org-protocol-default-template-key nil
   "The default template key to use.
 This is usually a single character string but can also be a
 string with two characters."
-  :group 'org-protocol
   :type '(choice (const nil) (string)))
 
 (defcustom org-protocol-data-separator "/+\\|\\?"
   "The default data separator to use.
 This should be a single regexp string."
-  :group 'org-protocol
   :version "24.4"
   :package-version '(Org . "8.0")
   :type 'regexp)
@@ -309,7 +304,8 @@ This should be a single regexp string."
 Emacsclient compresses double and triple slashes."
   (when (string-match "^\\([a-z]+\\):/" uri)
     (let* ((splitparts (split-string uri "/+")))
-      (setq uri (concat (car splitparts) "//" (mapconcat 'identity (cdr splitparts) "/")))))
+      (setq uri (concat (car splitparts) "//"
+                        (mapconcat #'identity (cdr splitparts) "/")))))
   uri)
 
 (defun org-protocol-split-data (data &optional unhexify separator)
@@ -549,10 +545,10 @@ Now template ?b will be used."
   "Convert QUERY key=value pairs in the URL to a property list."
   (when query
     (let ((plus-decoded (replace-regexp-in-string "\\+" " " query t t)))
-      (apply 'append (mapcar (lambda (x)
-			       (let ((c (split-string x "=")))
-				 (list (intern (concat ":" (car c))) (cadr c))))
-			     (split-string plus-decoded "&"))))))
+      (cl-mapcan (lambda (x)
+		   (let ((c (split-string x "=")))
+		     (list (intern (concat ":" (car c))) (cadr c))))
+		 (split-string plus-decoded "&")))))
 
 (defun org-protocol-open-source (fname)
   "Process an org-protocol://open-source?url= style URL with FNAME.
@@ -641,7 +637,7 @@ Old-style links such as \"protocol://sub-protocol://param1/param2\" are
 also recognized.
 
 If a matching protocol is found, the protocol is stripped from
-fname and the result is passed to the protocol function as the
+FNAME and the result is passed to the protocol function as the
 first parameter.  The second parameter will be non-nil if FNAME
 uses key=val&key2=val2-type arguments, or nil if FNAME uses
 val/val2-type arguments.  If the function returns nil, the
@@ -687,12 +683,12 @@ to deal with new-style links.")
                     (throw 'fname t))))))))
       fname)))
 
-(defadvice server-visit-files (before org-protocol-detect-protocol-server activate)
+(advice-add 'server-visit-files :around #'org--protocol-detect-protocol-server)
+(defun org--protocol-detect-protocol-server (orig-fun files client &rest args)
   "Advice server-visit-flist to call `org-protocol-modify-filename-for-protocol'."
   (let ((flist (if org-protocol-reverse-list-of-files
-                   (reverse  (ad-get-arg 0))
-                 (ad-get-arg 0)))
-        (client (ad-get-arg 1)))
+                   (reverse files)
+                 files)))
     (catch 'greedy
       (dolist (var flist)
 	;; `\' to `/' on windows.  FIXME: could this be done any better?
@@ -701,11 +697,16 @@ to deal with new-style links.")
 		       fname (member var flist)  client))
           (if (eq fname t) ;; greedy? We need the t return value.
               (progn
-                (ad-set-arg 0 nil)
+                ;; FIXME: Doesn't this just ignore all the files before
+                ;; this one (the remaining ones have been passed to
+                ;; `org-protocol-check-filename-for-protocol' but not
+                ;; the ones before).
+                (setq files nil)
                 (throw 'greedy t))
             (if (stringp fname) ;; probably filename
                 (setcar var fname)
-              (ad-set-arg 0 (delq var (ad-get-arg 0))))))))))
+              (setq files (delq var files)))))))
+    (apply orig-fun files client args)))
 
 ;;; Org specific functions:
 
diff --git a/lisp/org-tempo.el b/lisp/org-tempo.el
index b34007bf78..ed71431751 100644
--- a/lisp/org-tempo.el
+++ b/lisp/org-tempo.el
@@ -67,7 +67,6 @@ just like `org-structure-template-alist'.  The tempo snippet
 
 Do not use \"I\" as a KEY, as it is reserved for expanding
 \"#+include\"."
-  :group 'org-tempo
   :type '(repeat (cons (string :tag "Key")
 		       (string :tag "Keyword")))
   :package-version '(Org . "9.2"))
@@ -102,8 +101,8 @@ Tempo templates will be added."
 
 Go through `org-structure-template-alist' and
 `org-tempo-keywords-alist' and update tempo templates."
-  (mapc 'org--check-org-structure-template-alist '(org-structure-template-alist
-						   org-tempo-keywords-alist))
+  (mapc #'org--check-org-structure-template-alist '(org-structure-template-alist
+						    org-tempo-keywords-alist))
   (let ((keys (org-tempo--keys)))
     ;; Check for duplicated snippet keys and warn if any are found.
     (when (> (length keys) (length (delete-dups keys)))
@@ -176,8 +175,8 @@ didn't succeed."
 ;; Org Tempo is set up with each new Org buffer and potentially in the
 ;; current Org buffer.
 
-(add-hook 'org-mode-hook 'org-tempo-setup)
-(add-hook 'org-tab-before-tab-emulation-hook 'org-tempo-complete-tag)
+(add-hook 'org-mode-hook #'org-tempo-setup)
+(add-hook 'org-tab-before-tab-emulation-hook #'org-tempo-complete-tag)
 
 ;; Enable Org Tempo in all open Org buffers.
 (dolist (b (org-buffer-list 'files))
diff --git a/lisp/org.el b/lisp/org.el
index 5e3d0b3339..59714c1713 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -8577,14 +8577,15 @@ This will temporarily bind local variables that are typically bound in
 Org mode to the values they have in Org mode, and then interactively
 call CMD."
   (org-load-modules-maybe)
-  (let (binds)
+  (let (vars vals)
     (dolist (var (org-get-local-variables))
       (when (or (not (boundp (car var)))
 		(eq (symbol-value (car var))
 		    (default-value (car var))))
-	(push (list (car var) `(quote ,(cadr var))) binds)))
-    (eval `(let ,binds
-	     (call-interactively (quote ,cmd))))))
+	(push (car var) vars)
+	(push (cadr var) vals)))
+    (cl-progv vars vals
+      (call-interactively cmd))))
 
 (defun org-get-category (&optional pos force-refresh)
   "Get the category applying to position POS."
@@ -9513,7 +9514,7 @@ block can be inserted by pressing TAB after the string \"<KEY\"."
 In particular, check if the Org 9.2 format is used as opposed to
 previous format."
   (let ((elm (cl-remove-if-not (lambda (x) (listp (cdr x)))
-			       (or (eval checklist)
+			       (or (symbol-value checklist)
 				   org-structure-template-alist))))
     (when elm
       (org-display-warning
@@ -12804,7 +12805,7 @@ a *different* entry, you cannot use these techniques."
 	    ;; Get the right scope
 	    (cond
 	     ((and scope (listp scope) (symbolp (car scope)))
-	      (setq scope (eval scope)))
+	      (setq scope (eval scope t)))
 	     ((eq scope 'agenda)
 	      (setq scope (org-agenda-files t)))
 	     ((eq scope 'agenda-with-archives)
@@ -14625,7 +14626,7 @@ Unless KEEPDATE is non-nil, update `org-ans2' to the cursor date."
   (let ((sf (selected-frame))
 	(sw (selected-window)))
     (select-window (get-buffer-window "*Calendar*" t))
-    (eval form)
+    (eval form t)
     (when (and (not keepdate) (calendar-cursor-to-date))
       (let* ((date (calendar-cursor-to-date))
 	     (time (encode-time 0 0 0 (nth 1 date) (nth 0 date) (nth 2 date))))
@@ -15039,9 +15040,9 @@ D may be an absolute day number, or a calendar-type list (month day year)."
   (let* ((sexp `(let ((entry ,entry)
 		      (date ',d))
 		  ,(car (read-from-string sexp))))
-	 (result (if calendar-debug-sexp (eval sexp)
+	 (result (if calendar-debug-sexp (eval sexp t)
 		   (condition-case nil
-		       (eval sexp)
+		       (eval sexp t)
 		     (error
 		      (beep)
 		      (message "Bad sexp at line %d in %s: %s"
@@ -15974,25 +15975,29 @@ in Org mode.
     (cdlatex-compute-tables))
   (unless org-cdlatex-texmathp-advice-is-done
     (setq org-cdlatex-texmathp-advice-is-done t)
-    (defadvice texmathp (around org-math-always-on activate)
-      "Always return t in Org buffers.
+    (advice-add 'texmathp :around #'org--math-always-on)))
+
+(defun org--math-always-on (orig-fun &rest args)
+  "Always return t in Org buffers.
 This is because we want to insert math symbols without dollars even outside
 the LaTeX math segments.  If Org mode thinks that point is actually inside
 an embedded LaTeX fragment, let `texmathp' do its job.
 `\\[org-cdlatex-mode-map]'"
-      (interactive)
-      (let (p)
-	(cond
-	 ((not (derived-mode-p 'org-mode)) ad-do-it)
-	 ((eq this-command 'cdlatex-math-symbol)
-	  (setq ad-return-value t
-		texmathp-why '("cdlatex-math-symbol in org-mode" . 0)))
-	 (t
-	  (let ((p (org-inside-LaTeX-fragment-p)))
-	    (if (and p (member (car p) (plist-get org-format-latex-options :matchers)))
-		(setq ad-return-value t
-		      texmathp-why '("Org mode embedded math" . 0))
-	      (when p ad-do-it)))))))))
+  (interactive)
+  (cond
+   ((not (derived-mode-p 'org-mode)) (apply orig-fun args))
+   ((eq this-command 'cdlatex-math-symbol)
+    (setq texmathp-why '("cdlatex-math-symbol in org-mode" . 0))
+    t)
+   (t
+    (let ((p (org-inside-LaTeX-fragment-p)))
+      (when p ;; FIXME: Shouldn't we return t when `p' is nil?
+	(if (member (car p)
+	            (plist-get org-format-latex-options :matchers))
+	    (progn
+	      (setq texmathp-why '("Org mode embedded math" . 0))
+	      t)
+	  (apply orig-fun args)))))))
 
 (defun turn-on-org-cdlatex ()
   "Unconditionally turn on `org-cdlatex-mode'."
@@ -16952,8 +16957,8 @@ overwritten, and the table is not marked as requiring realignment."
       (call-interactively org-speed-command))
      ((functionp org-speed-command)
       (funcall org-speed-command))
-     ((and org-speed-command (listp org-speed-command))
-      (eval org-speed-command))
+     ((consp org-speed-command)
+      (eval org-speed-command t))
      (t (let (org-use-speed-commands)
 	  (call-interactively 'org-self-insert-command)))))
    ((and
@@ -18781,7 +18786,8 @@ such private information before sending the email.")
 			    (string-match "\\(-hook\\|-function\\)\\'" (symbol-name v)))
 		       (and
 			(get v 'custom-type) (get v 'standard-value)
-			(not (equal (symbol-value v) (eval (car (get v 'standard-value)))))))
+			(not (equal (symbol-value v)
+			            (eval (car (get v 'standard-value)) t)))))
 		   (push v list)))))
 	 (kill-buffer (get-buffer "*Warn about privacy*"))
 	 list))
diff --git a/lisp/ox-odt.el b/lisp/ox-odt.el
index 7f2e8ba47f..aa6e901223 100644
--- a/lisp/ox-odt.el
+++ b/lisp/ox-odt.el
@@ -362,7 +362,6 @@ When this option is turned on, `indent-region' is run on all
 component xml buffers before they are saved.  Turn this off for
 regular use.  Turn this on if you need to examine the xml
 visually."
-  :group 'org-export-odt
   :version "24.1"
   :type 'boolean)
 
@@ -399,7 +398,6 @@ with GNU ELPA tar or standard Emacs distribution."
   :type '(choice
 	  (const :tag "Not set" nil)
 	  (directory :tag "Schema directory"))
-  :group 'org-export-odt
   :version "24.1"
   :set
   (lambda (var value)
@@ -437,7 +435,6 @@ If unspecified, the file named \"OrgOdtContentTemplate.xml\"
 under `org-odt-styles-dir' is used."
   :type '(choice (const nil)
 		 (file))
-  :group 'org-export-odt
   :version "24.3")
 
 (defcustom org-odt-styles-file nil
@@ -471,7 +468,6 @@ a per-file basis.  For example,
 
 #+ODT_STYLES_FILE: \"/path/to/styles.xml\" or
 #+ODT_STYLES_FILE: (\"/path/to/file.ott\" (\"styles.xml\" \"image/hdr.png\"))."
-  :group 'org-export-odt
   :version "24.1"
   :type
   '(choice
@@ -486,7 +482,6 @@ a per-file basis.  For example,
 
 (defcustom org-odt-display-outline-level 2
   "Outline levels considered for enumerating captioned entities."
-  :group 'org-export-odt
   :version "24.4"
   :package-version '(Org . "8.0")
   :type 'integer)
@@ -516,7 +511,6 @@ specifiers are interpreted as below:
 %d output dir in full
 %D output dir as a URL.
 %x extra options as set in `org-odt-convert-capabilities'."
-  :group 'org-export-odt
   :version "24.1"
   :type
   '(choice
@@ -529,7 +523,6 @@ specifiers are interpreted as below:
   "Use this converter to convert from \"odt\" format to other formats.
 During customization, the list of converter names are populated
 from `org-odt-convert-processes'."
-  :group 'org-export-odt
   :version "24.1"
   :type '(choice :convert-widget
 		 (lambda (w)
@@ -591,7 +584,6 @@ format) to be converted to any of the export formats associated
 with that class.
 
 See default setting of this variable for a typical configuration."
-  :group 'org-export-odt
   :version "24.1"
   :type
   '(choice
@@ -618,7 +610,6 @@ variable, the list of valid values are populated based on
 
 You can set this option on per-file basis using file local
 values.  See Info node `(emacs) File Variables'."
-  :group 'org-export-odt
   :version "24.1"
   :type '(choice :convert-widget
 		 (lambda (w)
@@ -644,7 +635,6 @@ The function must accept two parameters:
 The function should return the string to be exported.
 
 The default value simply returns the value of CONTENTS."
-  :group 'org-export-odt
   :version "26.1"
   :package-version '(Org . "8.3")
   :type 'function)
@@ -664,7 +654,6 @@ TEXT      the main headline text (string).
 TAGS      the tags string, separated with colons (string or nil).
 
 The function result will be used as headline text."
-  :group 'org-export-odt
   :version "26.1"
   :package-version '(Org . "8.3")
   :type 'function)
@@ -685,7 +674,6 @@ The function must accept six parameters:
   CONTENTS  the contents of the inlinetask, as a string.
 
 The function should return the string to be exported."
-  :group 'org-export-odt
   :version "26.1"
   :package-version '(Org . "8.3")
   :type 'function)
@@ -712,7 +700,6 @@ nil            Ignore math snippets.
                be loaded.
 
 Any other symbol is a synonym for `mathjax'."
-  :group 'org-export-odt
   :version "24.4"
   :package-version '(Org . "8.0")
   :type '(choice
@@ -732,7 +719,6 @@ Any other symbol is a synonym for `mathjax'."
 A rule consists in an association whose key is the type of link
 to consider, and value is a regexp that will be matched against
 link's path."
-  :group 'org-export-odt
   :version "24.4"
   :package-version '(Org . "8.0")
   :type '(alist :key-type (string :tag "Type")
@@ -745,7 +731,6 @@ link's path."
 A rule consists in an association whose key is the type of link
 to consider, and value is a regexp that will be matched against
 link's path."
-  :group 'org-export-odt
   :version "26.1"
   :package-version '(Org . "8.3")
   :type '(alist :key-type (string :tag "Type")
@@ -756,7 +741,6 @@ link's path."
 Use this for sizing of embedded images.  See Info node `(org)
 Images in ODT export' for more information."
   :type 'float
-  :group 'org-export-odt
   :version "24.4"
   :package-version '(Org . "8.1"))
 
@@ -778,7 +762,6 @@ styles.xml already contains needed styles for colorizing to work.
 
 This variable is effective only if `org-odt-fontify-srcblocks' is
 turned on."
-  :group 'org-export-odt
   :version "24.1"
   :type 'boolean)
 
@@ -788,7 +771,6 @@ Turn this option on if you want to colorize the source code
 blocks in the exported file.  For colorization to work, you need
 to make available an enhanced version of `htmlfontify' library."
   :type 'boolean
-  :group 'org-export-odt
   :version "24.1")
 
 
@@ -873,7 +855,6 @@ implementation filed under `org-odt-get-table-cell-styles'.
 The TABLE-STYLE-NAME \"OrgEquation\" is used internally for
 formatting of numbered display equations.  Do not delete this
 style from the list."
-  :group 'org-export-odt
   :version "24.1"
   :type '(choice
           (const :tag "None" nil)
@@ -918,7 +899,6 @@ document by setting the default language and country either using
 the application UI or through a custom styles file.
 
 See `org-odt--build-date-styles' for implementation details."
-  :group 'org-export-odt
   :version "24.4"
   :package-version '(Org . "8.0")
   :type 'boolean)
@@ -2005,14 +1985,16 @@ information."
 
 ;;;; Latex Environment
 
-
 ;; (eval-after-load 'ox-odt '(ad-deactivate 'org-format-latex-as-mathml))
-;; (defadvice org-format-latex-as-mathml	; FIXME
-;;   (after org-odt-protect-latex-fragment activate)
+;; (advice-add 'org-format-latex-as-mathml	; FIXME
+;;   :around #'org--odt-protect-latex-fragment)
+;; (defun org--odt-protect-latex-fragment (orig-fun latex-frag &rest args)
 ;;   "Encode LaTeX fragment as XML.
 ;; Do this when translation to MathML fails."
-;;   (unless (> (length ad-return-value) 0)
-;;     (setq ad-return-value (org-odt--encode-plain-text (ad-get-arg 0)))))
+;;   (let ((retval (apply orig-fun latex-frag args)))
+;;     (if (> (length retval) 0)
+;;         retval
+;;       (org-odt--encode-plain-text latex-frag))))
 
 (defun org-odt-latex-environment (latex-environment _contents info)
   "Transcode a LATEX-ENVIRONMENT element from Org to ODT.

Re: Drop defadvice from Org

[Samuel Wales]

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 <monnier@iro.umontreal.ca> 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  <monnier@iro.umontreal.ca>
>
>     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 (&rest _)
>    "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 (&rest _)
> +  "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 (&rest _)
>    "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 'simple
> -  '(defadvice exchange-point-and-mark (after org-make-visible activate)
> -     "Make the point visible with `org-show-context'."
> -     (org-mark-jump-unhide)))
> -
> -(eval-after-load 'simple
> -  '(defadvice pop-global-mark (after org-make-visible activate)
> -     "Make the point visible with `org-show-context'."
> -     (org-mark-jump-unhide)))
> +(advice-add 'exchange-point-and-mark :after #'org-mark-jump-unhide)
> +(advice-add 'pop-global-mark :after #'org-mark-jump-unhide)
>
>  ;;;; Session
>
> diff --git a/lisp/org-ctags.el b/lisp/org-ctags.el
> index 6fc97ca399..59a08d0b54 100644
> --- a/lisp/org-ctags.el
> +++ b/lisp/org-ctags.el
> @@ -157,7 +157,6 @@ See the ctags documentation for more information.")
>  (defcustom org-ctags-path-to-ctags
>    (if (executable-find "ctags-exuberant") "ctags-exuberant" "ctags")
>    "Name of the ctags executable file."
> -  :group 'org-ctags
>    :version "24.1"
>    :type 'file)
>
> @@ -166,7 +165,6 @@ See the ctags documentation for more information.")
>      org-ctags-ask-rebuild-tags-file-then-find-tag
>      org-ctags-ask-append-topic)
>    "List of functions to be prepended to ORG-OPEN-LINK-FUNCTIONS by
> ORG-CTAGS."
> -  :group 'org-ctags
>    :version "24.1"
>    :type 'hook
>    :options '(org-ctags-find-tag
> @@ -188,7 +186,6 @@ Created as a local variable in each buffer.")
>    "Text to insert when creating a new org file via opening a hyperlink.
>  The following patterns are replaced in the string:
>      `%t' - replaced with the capitalized title of the hyperlink"
> -  :group 'org-ctags
>    :version "24.1"
>    :type 'string)
>
> @@ -207,7 +204,8 @@ The following patterns are replaced in the string:
>                    (visit-tags-table tags-filename))))))
>
>
> -(defadvice visit-tags-table (after org-ctags-load-tag-list activate
> compile)
> +(advice-add 'visit-tags-table :after #'org--ctags-load-tag-list)
> +(defun org--ctags-load-tag-list (&rest _)
>    (when (and org-ctags-enabled-p tags-file-name)
>      (setq-local org-ctags-tag-list
>  		(org-ctags-all-tags-in-current-tags-table))))
> @@ -295,8 +293,9 @@ The new topic will be titled NAME (or TITLE if
> supplied)."
>  ;;;; Misc interoperability with etags system
> =================================
>
>
> -(defadvice xref-find-definitions
> -    (before org-ctags-set-org-mark-before-finding-tag activate compile)
> +(advice-add 'xref-find-definitions :before
> +            #'org--ctags-set-org-mark-before-finding-tag)
> +(defun org--ctags-set-org-mark-before-finding-tag (&rest _)
>    "Before trying to find a tag, save our current position on org mark
> ring."
>    (save-excursion
>      (when (and (derived-mode-p 'org-mode) org-ctags-enabled-p)
> diff --git a/lisp/org-mouse.el b/lisp/org-mouse.el
> index 20c20acc32..2d8136b752 100644
> --- a/lisp/org-mouse.el
> +++ b/lisp/org-mouse.el
> @@ -580,15 +580,17 @@ This means, between the beginning of line and the
> point."
>    (insert text)
>    (beginning-of-line))
>
> -(defadvice dnd-insert-text (around org-mouse-dnd-insert-text activate)
> +(advice-add 'dnd-insert-text :around #'org--mouse-dnd-insert-text)
> +(defun org--mouse-dnd-insert-text (orig-fun window action text &rest args)
>    (if (derived-mode-p 'org-mode)
>        (org-mouse-insert-item text)
> -    ad-do-it))
> +    (apply orig-fun window action text args)))
>
> -(defadvice dnd-open-file (around org-mouse-dnd-open-file activate)
> +(advice-add 'dnd-open-file :around #'org--mouse-dnd-open-file)
> +(defun org--mouse-dnd-open-file (orig-fun uri &rest args)
>    (if (derived-mode-p 'org-mode)
>        (org-mouse-insert-item uri)
> -    ad-do-it))
> +    (apply orig-fun uri args)))
>
>  (defun org-mouse-match-closure (function)
>    (let ((match (match-data t)))
> @@ -894,15 +896,17 @@ This means, between the beginning of line and the
> point."
>                    (1 `(face nil keymap ,org-mouse-map mouse-face highlight)
> prepend)))
>                 t))
>
> -            (defadvice org-open-at-point (around org-mouse-open-at-point
> activate)
> -              (let ((context (org-context)))
> -                (cond
> -                 ((assq :headline-stars context) (org-cycle))
> -                 ((assq :checkbox context) (org-toggle-checkbox))
> -                 ((assq :item-bullet context)
> -                  (let ((org-cycle-include-plain-lists t)) (org-cycle)))
> -                 ((org-footnote-at-reference-p) nil)
> -                 (t ad-do-it))))))
> +            (advice-add 'org-open-at-point :around
> #'org--mouse-open-at-point)))
> +
> +(defun org--mouse-open-at-point (orig-fun &rest args)
> +  (let ((context (org-context)))
> +    (cond
> +     ((assq :headline-stars context) (org-cycle))
> +     ((assq :checkbox context) (org-toggle-checkbox))
> +     ((assq :item-bullet context)
> +      (let ((org-cycle-include-plain-lists t)) (org-cycle)))
> +     ((org-footnote-at-reference-p) nil)
> +     (t (apply orig-fun args)))))
>
>  (defun org-mouse-move-tree-start (_event)
>    (interactive "e")
> diff --git a/lisp/org-persist.el b/lisp/org-persist.el
> index 3b6a2d330a..dc14af817e 100644
> --- a/lisp/org-persist.el
> +++ b/lisp/org-persist.el
> @@ -179,7 +179,6 @@
>                                         cache-dir))
>                                     "org-persist/"))
>    "Directory where the data is stored."
> -  :group 'org-persist
>    :type 'directory)
>
>  (defcustom org-persist-remote-files 100
> @@ -931,8 +930,8 @@ Also, remove containers associated with non-existing
> files."
>        (message "Missing write access rights to org-persist-directory: %S"
>                 org-persist-directory)
>      (add-hook 'kill-emacs-hook #'org-persist-write-all)
> -    ;; `org-persist-gc' should run before `org-persist-write-all'.  So we
> are adding the
> -    ;; hook after `org-persist-write-all'.
> +    ;; `org-persist-gc' should run before `org-persist-write-all'.
> +    ;; So we are adding the hook after `org-persist-write-all'.
>      (add-hook 'kill-emacs-hook #'org-persist-gc)))
>
>  (add-hook 'after-init-hook #'org-persist-load-all)
> diff --git a/lisp/org-protocol.el b/lisp/org-protocol.el
> index 7c4de03bc2..1969f51fe3 100644
> --- a/lisp/org-protocol.el
> +++ b/lisp/org-protocol.el
> @@ -174,7 +174,6 @@ The filenames passed on the command line are passed to
> the emacs-server in
>  reverse order.  Set to t (default) to re-reverse the list, i.e. use the
>  sequence on the command line.  If nil, the sequence of the filenames is
>  unchanged."
> -  :group 'org-protocol
>    :type 'boolean)
>
>  (defcustom org-protocol-project-alist nil
> @@ -233,7 +232,6 @@ Example:
>  Consider using the interactive functions `org-protocol-create'
>  and `org-protocol-create-for-org' to help you filling this
>  variable with valid contents."
> -  :group 'org-protocol
>    :type 'alist)
>
>  (defcustom org-protocol-protocol-alist nil
> @@ -284,20 +282,17 @@ Here is an example:
>          (\"your-protocol\"
>           :protocol \"your-protocol\"
>           :function your-protocol-handler-function)))"
> -  :group 'org-protocol
>    :type '(alist))
>
>  (defcustom org-protocol-default-template-key nil
>    "The default template key to use.
>  This is usually a single character string but can also be a
>  string with two characters."
> -  :group 'org-protocol
>    :type '(choice (const nil) (string)))
>
>  (defcustom org-protocol-data-separator "/+\\|\\?"
>    "The default data separator to use.
>  This should be a single regexp string."
> -  :group 'org-protocol
>    :version "24.4"
>    :package-version '(Org . "8.0")
>    :type 'regexp)
> @@ -309,7 +304,8 @@ This should be a single regexp string."
>  Emacsclient compresses double and triple slashes."
>    (when (string-match "^\\([a-z]+\\):/" uri)
>      (let* ((splitparts (split-string uri "/+")))
> -      (setq uri (concat (car splitparts) "//" (mapconcat 'identity (cdr
> splitparts) "/")))))
> +      (setq uri (concat (car splitparts) "//"
> +                        (mapconcat #'identity (cdr splitparts) "/")))))
>    uri)
>
>  (defun org-protocol-split-data (data &optional unhexify separator)
> @@ -549,10 +545,10 @@ Now template ?b will be used."
>    "Convert QUERY key=value pairs in the URL to a property list."
>    (when query
>      (let ((plus-decoded (replace-regexp-in-string "\\+" " " query t t)))
> -      (apply 'append (mapcar (lambda (x)
> -			       (let ((c (split-string x "=")))
> -				 (list (intern (concat ":" (car c))) (cadr c))))
> -			     (split-string plus-decoded "&"))))))
> +      (cl-mapcan (lambda (x)
> +		   (let ((c (split-string x "=")))
> +		     (list (intern (concat ":" (car c))) (cadr c))))
> +		 (split-string plus-decoded "&")))))
>
>  (defun org-protocol-open-source (fname)
>    "Process an org-protocol://open-source?url= style URL with FNAME.
> @@ -641,7 +637,7 @@ Old-style links such as
> \"protocol://sub-protocol://param1/param2\" are
>  also recognized.
>
>  If a matching protocol is found, the protocol is stripped from
> -fname and the result is passed to the protocol function as the
> +FNAME and the result is passed to the protocol function as the
>  first parameter.  The second parameter will be non-nil if FNAME
>  uses key=val&key2=val2-type arguments, or nil if FNAME uses
>  val/val2-type arguments.  If the function returns nil, the
> @@ -687,12 +683,12 @@ to deal with new-style links.")
>                      (throw 'fname t))))))))
>        fname)))
>
> -(defadvice server-visit-files (before org-protocol-detect-protocol-server
> activate)
> +(advice-add 'server-visit-files :around
> #'org--protocol-detect-protocol-server)
> +(defun org--protocol-detect-protocol-server (orig-fun files client &rest
> args)
>    "Advice server-visit-flist to call
> `org-protocol-modify-filename-for-protocol'."
>    (let ((flist (if org-protocol-reverse-list-of-files
> -                   (reverse  (ad-get-arg 0))
> -                 (ad-get-arg 0)))
> -        (client (ad-get-arg 1)))
> +                   (reverse files)
> +                 files)))
>      (catch 'greedy
>        (dolist (var flist)
>  	;; `\' to `/' on windows.  FIXME: could this be done any better?
> @@ -701,11 +697,16 @@ to deal with new-style links.")
>  		       fname (member var flist)  client))
>            (if (eq fname t) ;; greedy? We need the t return value.
>                (progn
> -                (ad-set-arg 0 nil)
> +                ;; FIXME: Doesn't this just ignore all the files before
> +                ;; this one (the remaining ones have been passed to
> +                ;; `org-protocol-check-filename-for-protocol' but not
> +                ;; the ones before).
> +                (setq files nil)
>                  (throw 'greedy t))
>              (if (stringp fname) ;; probably filename
>                  (setcar var fname)
> -              (ad-set-arg 0 (delq var (ad-get-arg 0))))))))))
> +              (setq files (delq var files)))))))
> +    (apply orig-fun files client args)))
>
>  ;;; Org specific functions:
>
> diff --git a/lisp/org-tempo.el b/lisp/org-tempo.el
> index b34007bf78..ed71431751 100644
> --- a/lisp/org-tempo.el
> +++ b/lisp/org-tempo.el
> @@ -67,7 +67,6 @@ just like `org-structure-template-alist'.  The tempo
> snippet
>
>  Do not use \"I\" as a KEY, as it is reserved for expanding
>  \"#+include\"."
> -  :group 'org-tempo
>    :type '(repeat (cons (string :tag "Key")
>  		       (string :tag "Keyword")))
>    :package-version '(Org . "9.2"))
> @@ -102,8 +101,8 @@ Tempo templates will be added."
>
>  Go through `org-structure-template-alist' and
>  `org-tempo-keywords-alist' and update tempo templates."
> -  (mapc 'org--check-org-structure-template-alist
> '(org-structure-template-alist
> -						   org-tempo-keywords-alist))
> +  (mapc #'org--check-org-structure-template-alist
> '(org-structure-template-alist
> +						    org-tempo-keywords-alist))
>    (let ((keys (org-tempo--keys)))
>      ;; Check for duplicated snippet keys and warn if any are found.
>      (when (> (length keys) (length (delete-dups keys)))
> @@ -176,8 +175,8 @@ didn't succeed."
>  ;; Org Tempo is set up with each new Org buffer and potentially in the
>  ;; current Org buffer.
>
> -(add-hook 'org-mode-hook 'org-tempo-setup)
> -(add-hook 'org-tab-before-tab-emulation-hook 'org-tempo-complete-tag)
> +(add-hook 'org-mode-hook #'org-tempo-setup)
> +(add-hook 'org-tab-before-tab-emulation-hook #'org-tempo-complete-tag)
>
>  ;; Enable Org Tempo in all open Org buffers.
>  (dolist (b (org-buffer-list 'files))
> diff --git a/lisp/org.el b/lisp/org.el
> index 5e3d0b3339..59714c1713 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -8577,14 +8577,15 @@ This will temporarily bind local variables that are
> typically bound in
>  Org mode to the values they have in Org mode, and then interactively
>  call CMD."
>    (org-load-modules-maybe)
> -  (let (binds)
> +  (let (vars vals)
>      (dolist (var (org-get-local-variables))
>        (when (or (not (boundp (car var)))
>  		(eq (symbol-value (car var))
>  		    (default-value (car var))))
> -	(push (list (car var) `(quote ,(cadr var))) binds)))
> -    (eval `(let ,binds
> -	     (call-interactively (quote ,cmd))))))
> +	(push (car var) vars)
> +	(push (cadr var) vals)))
> +    (cl-progv vars vals
> +      (call-interactively cmd))))
>
>  (defun org-get-category (&optional pos force-refresh)
>    "Get the category applying to position POS."
> @@ -9513,7 +9514,7 @@ block can be inserted by pressing TAB after the string
> \"<KEY\"."
>  In particular, check if the Org 9.2 format is used as opposed to
>  previous format."
>    (let ((elm (cl-remove-if-not (lambda (x) (listp (cdr x)))
> -			       (or (eval checklist)
> +			       (or (symbol-value checklist)
>  				   org-structure-template-alist))))
>      (when elm
>        (org-display-warning
> @@ -12804,7 +12805,7 @@ a *different* entry, you cannot use these
> techniques."
>  	    ;; Get the right scope
>  	    (cond
>  	     ((and scope (listp scope) (symbolp (car scope)))
> -	      (setq scope (eval scope)))
> +	      (setq scope (eval scope t)))
>  	     ((eq scope 'agenda)
>  	      (setq scope (org-agenda-files t)))
>  	     ((eq scope 'agenda-with-archives)
> @@ -14625,7 +14626,7 @@ Unless KEEPDATE is non-nil, update `org-ans2' to the
> cursor date."
>    (let ((sf (selected-frame))
>  	(sw (selected-window)))
>      (select-window (get-buffer-window "*Calendar*" t))
> -    (eval form)
> +    (eval form t)
>      (when (and (not keepdate) (calendar-cursor-to-date))
>        (let* ((date (calendar-cursor-to-date))
>  	     (time (encode-time 0 0 0 (nth 1 date) (nth 0 date) (nth 2 date))))
> @@ -15039,9 +15040,9 @@ D may be an absolute day number, or a calendar-type
> list (month day year)."
>    (let* ((sexp `(let ((entry ,entry)
>  		      (date ',d))
>  		  ,(car (read-from-string sexp))))
> -	 (result (if calendar-debug-sexp (eval sexp)
> +	 (result (if calendar-debug-sexp (eval sexp t)
>  		   (condition-case nil
> -		       (eval sexp)
> +		       (eval sexp t)
>  		     (error
>  		      (beep)
>  		      (message "Bad sexp at line %d in %s: %s"
> @@ -15974,25 +15975,29 @@ in Org mode.
>      (cdlatex-compute-tables))
>    (unless org-cdlatex-texmathp-advice-is-done
>      (setq org-cdlatex-texmathp-advice-is-done t)
> -    (defadvice texmathp (around org-math-always-on activate)
> -      "Always return t in Org buffers.
> +    (advice-add 'texmathp :around #'org--math-always-on)))
> +
> +(defun org--math-always-on (orig-fun &rest args)
> +  "Always return t in Org buffers.
>  This is because we want to insert math symbols without dollars even
> outside
>  the LaTeX math segments.  If Org mode thinks that point is actually inside
>  an embedded LaTeX fragment, let `texmathp' do its job.
>  `\\[org-cdlatex-mode-map]'"
> -      (interactive)
> -      (let (p)
> -	(cond
> -	 ((not (derived-mode-p 'org-mode)) ad-do-it)
> -	 ((eq this-command 'cdlatex-math-symbol)
> -	  (setq ad-return-value t
> -		texmathp-why '("cdlatex-math-symbol in org-mode" . 0)))
> -	 (t
> -	  (let ((p (org-inside-LaTeX-fragment-p)))
> -	    (if (and p (member (car p) (plist-get org-format-latex-options
> :matchers)))
> -		(setq ad-return-value t
> -		      texmathp-why '("Org mode embedded math" . 0))
> -	      (when p ad-do-it)))))))))
> +  (interactive)
> +  (cond
> +   ((not (derived-mode-p 'org-mode)) (apply orig-fun args))
> +   ((eq this-command 'cdlatex-math-symbol)
> +    (setq texmathp-why '("cdlatex-math-symbol in org-mode" . 0))
> +    t)
> +   (t
> +    (let ((p (org-inside-LaTeX-fragment-p)))
> +      (when p ;; FIXME: Shouldn't we return t when `p' is nil?
> +	(if (member (car p)
> +	            (plist-get org-format-latex-options :matchers))
> +	    (progn
> +	      (setq texmathp-why '("Org mode embedded math" . 0))
> +	      t)
> +	  (apply orig-fun args)))))))
>
>  (defun turn-on-org-cdlatex ()
>    "Unconditionally turn on `org-cdlatex-mode'."
> @@ -16952,8 +16957,8 @@ overwritten, and the table is not marked as
> requiring realignment."
>        (call-interactively org-speed-command))
>       ((functionp org-speed-command)
>        (funcall org-speed-command))
> -     ((and org-speed-command (listp org-speed-command))
> -      (eval org-speed-command))
> +     ((consp org-speed-command)
> +      (eval org-speed-command t))
>       (t (let (org-use-speed-commands)
>  	  (call-interactively 'org-self-insert-command)))))
>     ((and
> @@ -18781,7 +18786,8 @@ such private information before sending the
> email.")
>  			    (string-match "\\(-hook\\|-function\\)\\'" (symbol-name v)))
>  		       (and
>  			(get v 'custom-type) (get v 'standard-value)
> -			(not (equal (symbol-value v) (eval (car (get v 'standard-value)))))))
> +			(not (equal (symbol-value v)
> +			            (eval (car (get v 'standard-value)) t)))))
>  		   (push v list)))))
>  	 (kill-buffer (get-buffer "*Warn about privacy*"))
>  	 list))
> diff --git a/lisp/ox-odt.el b/lisp/ox-odt.el
> index 7f2e8ba47f..aa6e901223 100644
> --- a/lisp/ox-odt.el
> +++ b/lisp/ox-odt.el
> @@ -362,7 +362,6 @@ When this option is turned on, `indent-region' is run on
> all
>  component xml buffers before they are saved.  Turn this off for
>  regular use.  Turn this on if you need to examine the xml
>  visually."
> -  :group 'org-export-odt
>    :version "24.1"
>    :type 'boolean)
>
> @@ -399,7 +398,6 @@ with GNU ELPA tar or standard Emacs distribution."
>    :type '(choice
>  	  (const :tag "Not set" nil)
>  	  (directory :tag "Schema directory"))
> -  :group 'org-export-odt
>    :version "24.1"
>    :set
>    (lambda (var value)
> @@ -437,7 +435,6 @@ If unspecified, the file named
> \"OrgOdtContentTemplate.xml\"
>  under `org-odt-styles-dir' is used."
>    :type '(choice (const nil)
>  		 (file))
> -  :group 'org-export-odt
>    :version "24.3")
>
>  (defcustom org-odt-styles-file nil
> @@ -471,7 +468,6 @@ a per-file basis.  For example,
>
>  #+ODT_STYLES_FILE: \"/path/to/styles.xml\" or
>  #+ODT_STYLES_FILE: (\"/path/to/file.ott\" (\"styles.xml\"
> \"image/hdr.png\"))."
> -  :group 'org-export-odt
>    :version "24.1"
>    :type
>    '(choice
> @@ -486,7 +482,6 @@ a per-file basis.  For example,
>
>  (defcustom org-odt-display-outline-level 2
>    "Outline levels considered for enumerating captioned entities."
> -  :group 'org-export-odt
>    :version "24.4"
>    :package-version '(Org . "8.0")
>    :type 'integer)
> @@ -516,7 +511,6 @@ specifiers are interpreted as below:
>  %d output dir in full
>  %D output dir as a URL.
>  %x extra options as set in `org-odt-convert-capabilities'."
> -  :group 'org-export-odt
>    :version "24.1"
>    :type
>    '(choice
> @@ -529,7 +523,6 @@ specifiers are interpreted as below:
>    "Use this converter to convert from \"odt\" format to other formats.
>  During customization, the list of converter names are populated
>  from `org-odt-convert-processes'."
> -  :group 'org-export-odt
>    :version "24.1"
>    :type '(choice :convert-widget
>  		 (lambda (w)
> @@ -591,7 +584,6 @@ format) to be converted to any of the export formats
> associated
>  with that class.
>
>  See default setting of this variable for a typical configuration."
> -  :group 'org-export-odt
>    :version "24.1"
>    :type
>    '(choice
> @@ -618,7 +610,6 @@ variable, the list of valid values are populated based
> on
>
>  You can set this option on per-file basis using file local
>  values.  See Info node `(emacs) File Variables'."
> -  :group 'org-export-odt
>    :version "24.1"
>    :type '(choice :convert-widget
>  		 (lambda (w)
> @@ -644,7 +635,6 @@ The function must accept two parameters:
>  The function should return the string to be exported.
>
>  The default value simply returns the value of CONTENTS."
> -  :group 'org-export-odt
>    :version "26.1"
>    :package-version '(Org . "8.3")
>    :type 'function)
> @@ -664,7 +654,6 @@ TEXT      the main headline text (string).
>  TAGS      the tags string, separated with colons (string or nil).
>
>  The function result will be used as headline text."
> -  :group 'org-export-odt
>    :version "26.1"
>    :package-version '(Org . "8.3")
>    :type 'function)
> @@ -685,7 +674,6 @@ The function must accept six parameters:
>    CONTENTS  the contents of the inlinetask, as a string.
>
>  The function should return the string to be exported."
> -  :group 'org-export-odt
>    :version "26.1"
>    :package-version '(Org . "8.3")
>    :type 'function)
> @@ -712,7 +700,6 @@ nil            Ignore math snippets.
>                 be loaded.
>
>  Any other symbol is a synonym for `mathjax'."
> -  :group 'org-export-odt
>    :version "24.4"
>    :package-version '(Org . "8.0")
>    :type '(choice
> @@ -732,7 +719,6 @@ Any other symbol is a synonym for `mathjax'."
>  A rule consists in an association whose key is the type of link
>  to consider, and value is a regexp that will be matched against
>  link's path."
> -  :group 'org-export-odt
>    :version "24.4"
>    :package-version '(Org . "8.0")
>    :type '(alist :key-type (string :tag "Type")
> @@ -745,7 +731,6 @@ link's path."
>  A rule consists in an association whose key is the type of link
>  to consider, and value is a regexp that will be matched against
>  link's path."
> -  :group 'org-export-odt
>    :version "26.1"
>    :package-version '(Org . "8.3")
>    :type '(alist :key-type (string :tag "Type")
> @@ -756,7 +741,6 @@ link's path."
>  Use this for sizing of embedded images.  See Info node `(org)
>  Images in ODT export' for more information."
>    :type 'float
> -  :group 'org-export-odt
>    :version "24.4"
>    :package-version '(Org . "8.1"))
>
> @@ -778,7 +762,6 @@ styles.xml already contains needed styles for colorizing
> to work.
>
>  This variable is effective only if `org-odt-fontify-srcblocks' is
>  turned on."
> -  :group 'org-export-odt
>    :version "24.1"
>    :type 'boolean)
>
> @@ -788,7 +771,6 @@ Turn this option on if you want to colorize the source
> code
>  blocks in the exported file.  For colorization to work, you need
>  to make available an enhanced version of `htmlfontify' library."
>    :type 'boolean
> -  :group 'org-export-odt
>    :version "24.1")
>
>
> @@ -873,7 +855,6 @@ implementation filed under
> `org-odt-get-table-cell-styles'.
>  The TABLE-STYLE-NAME \"OrgEquation\" is used internally for
>  formatting of numbered display equations.  Do not delete this
>  style from the list."
> -  :group 'org-export-odt
>    :version "24.1"
>    :type '(choice
>            (const :tag "None" nil)
> @@ -918,7 +899,6 @@ document by setting the default language and country
> either using
>  the application UI or through a custom styles file.
>
>  See `org-odt--build-date-styles' for implementation details."
> -  :group 'org-export-odt
>    :version "24.4"
>    :package-version '(Org . "8.0")
>    :type 'boolean)
> @@ -2005,14 +1985,16 @@ information."
>
>  ;;;; Latex Environment
>
> -
>  ;; (eval-after-load 'ox-odt '(ad-deactivate 'org-format-latex-as-mathml))
> -;; (defadvice org-format-latex-as-mathml	; FIXME
> -;;   (after org-odt-protect-latex-fragment activate)
> +;; (advice-add 'org-format-latex-as-mathml	; FIXME
> +;;   :around #'org--odt-protect-latex-fragment)
> +;; (defun org--odt-protect-latex-fragment (orig-fun latex-frag &rest args)
>  ;;   "Encode LaTeX fragment as XML.
>  ;; Do this when translation to MathML fails."
> -;;   (unless (> (length ad-return-value) 0)
> -;;     (setq ad-return-value (org-odt--encode-plain-text (ad-get-arg
> 0)))))
> +;;   (let ((retval (apply orig-fun latex-frag args)))
> +;;     (if (> (length retval) 0)
> +;;         retval
> +;;       (org-odt--encode-plain-text latex-frag))))
>
>  (defun org-odt-latex-environment (latex-environment _contents info)
>    "Transcode a LATEX-ENVIRONMENT element from Org to ODT.
>
>
>


-- 
The Kafka Pandemic

A blog about science, health, human rights, and misopathy:
https://thekafkapandemic.blogspot.com

Re: Drop defadvice from Org

[Stefan Monnier]

> 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

[Samuel Wales]

On 3/31/22, Stefan Monnier <monnier@iro.umontreal.ca> 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

[Stefan Monnier]

> 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

[Bastien Guerry]

Hi Stefan,

Stefan Monnier <monnier@iro.umontreal.ca> 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

[Ihor Radchenko]

Bastien Guerry <bzg@gnu.org> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> 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

[Bastien]

Hi Ihor,

Ihor Radchenko <yantar92@gmail.com> writes:

> The change in (eval ...) call inside org-diary-sexp-entry broke sexp
> timestamps. See orgmode.org/list/875ynnojvf.fsf@localhost

Fixed, thanks!

-- 
 Bastien