* Re: master 9ade7ea: Fix Eldoc problem when loading on Emacs 26.3 [not found] ` <20200710004936.2935520A27@vcs0.savannah.gnu.org> @ 2020-07-10 16:09 ` Glenn Morris 2020-07-10 17:09 ` João Távora 0 siblings, 1 reply; 11+ messages in thread From: Glenn Morris @ 2020-07-10 16:09 UTC (permalink / raw) To: emacs-devel; +Cc: João Távora > branch: master > commit 9ade7ea7b77ec40c16deb4dff139ce7127a703e2 This change causes 194 test failures. Ref eg https://hydra.nixos.org/build/123765785 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: master 9ade7ea: Fix Eldoc problem when loading on Emacs 26.3 2020-07-10 16:09 ` master 9ade7ea: Fix Eldoc problem when loading on Emacs 26.3 Glenn Morris @ 2020-07-10 17:09 ` João Távora 2020-07-10 17:36 ` Noam Postavsky 0 siblings, 1 reply; 11+ messages in thread From: João Távora @ 2020-07-10 17:09 UTC (permalink / raw) To: Glenn Morris; +Cc: emacs-devel On Fri, Jul 10, 2020 at 5:09 PM Glenn Morris <rgm@gnu.org> wrote: > > > > branch: master > > commit 9ade7ea7b77ec40c16deb4dff139ce7127a703e2 > > This change causes 194 test failures. > Ref eg https://hydra.nixos.org/build/123765785 Reverted it, will have to find some other way. makunbound, as Noam suggested, doesn't work. Is there an easy way to "unlocalize" a symbol? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: master 9ade7ea: Fix Eldoc problem when loading on Emacs 26.3 2020-07-10 17:09 ` João Távora @ 2020-07-10 17:36 ` Noam Postavsky 2020-07-10 17:54 ` João Távora 0 siblings, 1 reply; 11+ messages in thread From: Noam Postavsky @ 2020-07-10 17:36 UTC (permalink / raw) To: João Távora; +Cc: Glenn Morris, emacs-devel On Fri, 10 Jul 2020 at 13:10, João Távora <joaotavora@gmail.com> wrote: > makunbound, as Noam suggested, doesn't work. > > Is there an easy way to "unlocalize" a symbol? Oh, maybe kill-local-variable? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: master 9ade7ea: Fix Eldoc problem when loading on Emacs 26.3 2020-07-10 17:36 ` Noam Postavsky @ 2020-07-10 17:54 ` João Távora 2020-07-10 19:57 ` João Távora 0 siblings, 1 reply; 11+ messages in thread From: João Távora @ 2020-07-10 17:54 UTC (permalink / raw) To: Noam Postavsky; +Cc: Glenn Morris, emacs-devel Noam Postavsky <npostavs@gmail.com> writes: > On Fri, 10 Jul 2020 at 13:10, João Távora <joaotavora@gmail.com> wrote: > >> makunbound, as Noam suggested, doesn't work. >> >> Is there an easy way to "unlocalize" a symbol? > > Oh, maybe kill-local-variable? No, that doesn't work either. I was looking at data.c and alloc.c and apparently the only way is really to unintern and re-intern the symbol. But of course that seems to break code that you've loaded before and that expects that symbol in its pre-uninterned state. I've even tried `garbage-collect` to see if I could trigger the sym->u.s.redirect = SYMBOL_PLAINVAL from Elisp, but it seems hard. So I'm a bit lost for options to be honest. The problem here, seems to be the fact that eldoc.el is pre-loaded and somehwere in that process the eldoc-documentation-function variable is localized in some buffer. Of course, the problem only manifests itself in practice when you're loading eldoc via package.el in previous Emacs versions, so maybe there's some way to unintern the symbol at package-initialize time. But that probably would break something, too. Maybe come up with a better idea, or maybe, like Stefan suggests, don't make it an alias at all, just deprecate it and keep using it instead of eldoc-documentation-strategy when we find it to be bound. João ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: master 9ade7ea: Fix Eldoc problem when loading on Emacs 26.3 2020-07-10 17:54 ` João Távora @ 2020-07-10 19:57 ` João Távora 2020-07-10 21:35 ` Stefan Monnier 0 siblings, 1 reply; 11+ messages in thread From: João Távora @ 2020-07-10 19:57 UTC (permalink / raw) To: Noam Postavsky; +Cc: Glenn Morris, emacs-devel João Távora <joaotavora@gmail.com> writes: > Noam Postavsky <npostavs@gmail.com> writes: >>> makunbound, as Noam suggested, doesn't work. >>> Is there an easy way to "unlocalize" a symbol? >> Oh, maybe kill-local-variable? > > No, that doesn't work either. I was looking at data.c and alloc.c and > Maybe come up with a better idea, or maybe, like Stefan suggests, don't > make it an alias at all, just deprecate it and keep using it instead of > eldoc-documentation-strategy when we find it to be bound. That sort of works, but it forces extensions that do want to "Package-Require" eldoc to remember to unbind eldoc-documentation-function if they are loaded on older Emacs versions, which is undesirable. So I think a better plan is just to reverse the variable alias on older Emacs versions. Attaching the patch that does this: commit b7019ed5e76dda40642db1b20d3dd1484b2536e6 Author: João Távora <joaotavora@gmail.com> Date: Fri Jul 10 20:49:54 2020 +0100 Sort out Eldoc backward compatibility of eldoc-documentation-function Use eldoc--documentation-strategy-defcustom to decide in which direction to make the variable alias, depending on the Emacs version. * lisp/emacs-lisp/eldoc.el (eldoc--documentation-strategy-defcustom): Helper macro. (eldoc-documentation-strategy, eldoc-documentation-function): Use it. (Version): Bump to 1.5.0 diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el index 7964c4c45a..ccae09a906 100644 --- a/lisp/emacs-lisp/eldoc.el +++ b/lisp/emacs-lisp/eldoc.el @@ -5,7 +5,7 @@ ;; Author: Noah Friedman <friedman@splode.com> ;; Keywords: extensions ;; Created: 1995-10-06 -;; Version: 1.4.0 +;; Version: 1.5.0 ;; Package-Requires: ((emacs "26.3")) ;; This is a GNU ELPA :core package. Avoid functionality that is not @@ -535,10 +535,27 @@ eldoc-documentation-enthusiast (if (stringp str) (funcall callback str)) nil)))) -(define-obsolete-variable-alias 'eldoc-documentation-function - 'eldoc-documentation-strategy "eldoc-1.1.0") - -(defcustom eldoc-documentation-strategy #'eldoc-documentation-default +;; JT@2020-07-10: Eldoc is pre-loaded, so in in Emacs < 28 we can't +;; make the "old" `eldoc-documentation-function' point to the new +;; `eldoc-documentation-strategy', so we do the reverse. This allows +;; for Eldoc to be loaded in those older Emacs versions and work with +;; whomever (major-modes, extensions, ueser) sets one of the other +;; variable. +(defmacro eldoc--documentation-strategy-defcustom + (main secondary value docstring &rest more) + "Defcustom helper macro for sorting `eldoc-documentation-strategy'." + (declare (indent 2)) + `(if (< emacs-major-version 28) + (progn + (defcustom ,secondary ,value ,docstring ,@more) + (defvaralias ',main ',secondary ,docstring)) + (progn + (defcustom ,main ,value ,docstring ,@more) + (defvaralias ',secondary ',main ,docstring)))) + +(eldoc--documentation-strategy-defcustom eldoc-documentation-strategy + eldoc-documentation-function + #'eldoc-documentation-default "How to collect and organize results of `eldoc-documentation-functions'. This variable controls how `eldoc-documentation-functions', which ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: master 9ade7ea: Fix Eldoc problem when loading on Emacs 26.3 2020-07-10 19:57 ` João Távora @ 2020-07-10 21:35 ` Stefan Monnier 2020-07-10 21:58 ` João Távora 0 siblings, 1 reply; 11+ messages in thread From: Stefan Monnier @ 2020-07-10 21:35 UTC (permalink / raw) To: João Távora; +Cc: Glenn Morris, Noam Postavsky, emacs-devel >> Maybe come up with a better idea, or maybe, like Stefan suggests, don't >> make it an alias at all, just deprecate it and keep using it instead of >> eldoc-documentation-strategy when we find it to be bound. > > That sort of works, but it forces extensions that do want to > "Package-Require" eldoc to remember to unbind > eldoc-documentation-function if they are loaded on older Emacs versions, > which is undesirable. Maybe use a rule like: (if (and (buffer-local-p 'eldoc-documentation-function) (not (buffer-local-p 'eldoc-documentation-functions))) <Use `eldoc-documentation-function`> <Use `eldoc-documentation-functions`> -- Stefan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: master 9ade7ea: Fix Eldoc problem when loading on Emacs 26.3 2020-07-10 21:35 ` Stefan Monnier @ 2020-07-10 21:58 ` João Távora 2020-07-10 22:16 ` Stefan Monnier 0 siblings, 1 reply; 11+ messages in thread From: João Távora @ 2020-07-10 21:58 UTC (permalink / raw) To: Stefan Monnier; +Cc: Glenn Morris, Noam Postavsky, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >>> Maybe come up with a better idea, or maybe, like Stefan suggests, don't >>> make it an alias at all, just deprecate it and keep using it instead of >>> eldoc-documentation-strategy when we find it to be bound. >> >> That sort of works, but it forces extensions that do want to >> "Package-Require" eldoc to remember to unbind >> eldoc-documentation-function if they are loaded on older Emacs versions, >> which is undesirable. > > Maybe use a rule like: > > (if (and (buffer-local-p 'eldoc-documentation-function) > (not (buffer-local-p 'eldoc-documentation-functions))) > <Use `eldoc-documentation-function`> > <Use `eldoc-documentation-functions`> Sounds a bit complicated for me, especially since there's already a eldoc--supported-p which does similar heuristics. Wouldn't it break in some older Emacs version that loads the new Eldoc and has some global value in eldoc-documentation-functions (say, a spell-checking eldoc backend)? For now, I'd like to go with the alias/reverse-alias technique, which is the simplest, I think, for this simple problem. I've tested it and it works correctly. Have you seen the patch? Do you see any blockers there? We can move on to some more sophisticated version later. João ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: master 9ade7ea: Fix Eldoc problem when loading on Emacs 26.3 2020-07-10 21:58 ` João Távora @ 2020-07-10 22:16 ` Stefan Monnier 2020-07-10 22:41 ` João Távora 0 siblings, 1 reply; 11+ messages in thread From: Stefan Monnier @ 2020-07-10 22:16 UTC (permalink / raw) To: João Távora; +Cc: Glenn Morris, Noam Postavsky, emacs-devel >>>> Maybe come up with a better idea, or maybe, like Stefan suggests, don't >>>> make it an alias at all, just deprecate it and keep using it instead of >>>> eldoc-documentation-strategy when we find it to be bound. >>> That sort of works, but it forces extensions that do want to >>> "Package-Require" eldoc to remember to unbind >>> eldoc-documentation-function if they are loaded on older Emacs versions, >>> which is undesirable. No. If they're only written for the old API they set `eldoc-documentation-function` and live in blissful ignorance of what they miss. If they're only written for the new API, they add-hook to `eldoc-documentation-functions` and live in bliss. And if they want to support both they do (if (boundp 'eldoc-documentation-functions) (add-hook 'eldoc-documentation-functions ...) (setq eldoc-documentation-function ...)) >> Maybe use a rule like: >> >> (if (and (buffer-local-p 'eldoc-documentation-function) >> (not (buffer-local-p 'eldoc-documentation-functions))) >> <Use `eldoc-documentation-function`> >> <Use `eldoc-documentation-functions`> > > Sounds a bit complicated for me, especially since there's already a > eldoc--supported-p which does similar heuristics. But that's one of the other benefits. It lets us distinguish in `eldoc--supported-p` the case where `eldoc-documentation-strategy` was set from the case where `eldoc-documentation-function` was set. > Wouldn't it break in some older Emacs version that loads the new Eldoc > and has some global value in eldoc-documentation-functions (say, > a spell-checking eldoc backend)? The above `buffer-local-p` tests would be in eldoc.el, not in the client packages, so I don't see what scenario you're worried about. And of course the presence of a global value doesn't affect `buffer-local-p` anyway, so ... I guess I must be missing something? Stefan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: master 9ade7ea: Fix Eldoc problem when loading on Emacs 26.3 2020-07-10 22:16 ` Stefan Monnier @ 2020-07-10 22:41 ` João Távora 2020-07-10 23:44 ` Stefan Monnier 0 siblings, 1 reply; 11+ messages in thread From: João Távora @ 2020-07-10 22:41 UTC (permalink / raw) To: Stefan Monnier; +Cc: Glenn Morris, Noam Postavsky, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >>>>> Maybe come up with a better idea, or maybe, like Stefan suggests, don't >>>>> make it an alias at all, just deprecate it and keep using it instead of >>>>> eldoc-documentation-strategy when we find it to be bound. >>>> That sort of works, but it forces extensions that do want to >>>> "Package-Require" eldoc to remember to unbind >>>> eldoc-documentation-function if they are loaded on older Emacs versions, >>>> which is undesirable. > > No. If they're only written for the old API they set > `eldoc-documentation-function` and live in blissful ignorance of what > they miss. If they're only written for the new API, they add-hook to > `eldoc-documentation-functions` and live in bliss. I was describing my own earlier solution, by the way, where I'm pretty sure that would happen. > And if they want to support both they do > > (if (boundp 'eldoc-documentation-functions) > (add-hook 'eldoc-documentation-functions ...) > (setq eldoc-documentation-function ...)) >>> Maybe use a rule like: >>> >>> (if (and (buffer-local-p 'eldoc-documentation-function) >>> (not (buffer-local-p 'eldoc-documentation-functions))) >>> <Use `eldoc-documentation-function`> >>> <Use `eldoc-documentation-functions`> >> >> Sounds a bit complicated for me, especially since there's already a >> eldoc--supported-p which does similar heuristics. > > But that's one of the other benefits. It lets us distinguish in > `eldoc--supported-p` the case where `eldoc-documentation-strategy` was > set from the case where `eldoc-documentation-function` was set. > >> Wouldn't it break in some older Emacs version that loads the new Eldoc >> and has some global value in eldoc-documentation-functions (say, >> a spell-checking eldoc backend)? > > The above `buffer-local-p` tests would be in eldoc.el, not in the client > packages, so I don't see what scenario you're worried about. > And of course the presence of a global value doesn't affect > `buffer-local-p` anyway, so ... I guess I must be missing something? Maybe I am, surely. This is complicated, and I'm probably not seeing through the complexity as well as you are. If you want to do these changes I won't object, but I'd probably mess them up. At least for now, I really do think the best way is to alias the two variables. Whichever way we alias them, it's just as if I never renamed the singular "e-d-function" to "e-d-strategy" at all, which is, after all, a much safer bet. I've even thought about dropping the whole "rename function to strategy" idea completely, which was just a cosmetic way to escape the awful mind-bending confusion between "e-d-function" singular and "e-d-functions" plural. So I think I'm going to go with the simple alias patch, unless you (or someone) strongly opposes. João ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: master 9ade7ea: Fix Eldoc problem when loading on Emacs 26.3 2020-07-10 22:41 ` João Távora @ 2020-07-10 23:44 ` Stefan Monnier 2020-07-11 8:06 ` João Távora 0 siblings, 1 reply; 11+ messages in thread From: Stefan Monnier @ 2020-07-10 23:44 UTC (permalink / raw) To: João Távora; +Cc: Glenn Morris, Noam Postavsky, emacs-devel > all, a much safer bet. I've even thought about dropping the whole > "rename function to strategy" idea completely, which was just a cosmetic > way to escape the awful mind-bending confusion between "e-d-function" > singular and "e-d-functions" plural. The reason why I was happy to see you use a different name is to make it possible for eldoc--supported-p to distinguish the case where something changed the -strategy var (which shouldn't affect eldoc--supported-p) from the case where something changed the -function var. Stefan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: master 9ade7ea: Fix Eldoc problem when loading on Emacs 26.3 2020-07-10 23:44 ` Stefan Monnier @ 2020-07-11 8:06 ` João Távora 0 siblings, 0 replies; 11+ messages in thread From: João Távora @ 2020-07-11 8:06 UTC (permalink / raw) To: Stefan Monnier; +Cc: Glenn Morris, Noam Postavsky, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> all, a much safer bet. I've even thought about dropping the whole >> "rename function to strategy" idea completely, which was just a cosmetic >> way to escape the awful mind-bending confusion between "e-d-function" >> singular and "e-d-functions" plural. > > The reason why I was happy to see you use a different name is to make it > possible for eldoc--supported-p to distinguish the case where something > changed the -strategy var (which shouldn't affect > eldoc--supported-p) from the case where something changed > the -function var. Yes, I know that now, and I don't oppose it. I just don't feel capacitated enough to separated the two things without mixing myself up :-). I drafted up this patch yesterday, but it's got this problem I mentioned. Maybe you can fix it. In the meantime, I'm going to go with the alias. João commit c40761d683dcf70fe7f404b675a3a2631edfeaf0 Author: João Távora <joaotavora@gmail.com> Date: Fri Jul 10 19:11:01 2020 +0100 Adjust Eldoc backward compatibility of eldoc-documentation-function Making a variable alias doesn't work because that variable is preloaded and localized. So just keep the eldoc-documentation-function, make it obsolete, and let it be used instead of strategy if it is found to be non-nil (either locally or globally). * lisp/emacs-lisp/eldoc.el (eldoc--eval-expression-setup): Set eldoc-documentation-function to nil. (eldoc-documentation-function): Reinstate variable. (eldoc--documentation-strategy): New helper function. (eldoc--invoke-strategy): Use eldoc--documentation-strategy. (eldoc--supported-p): Reowkr (Version): Bump to 1.5.0 eldoc--supported-p diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el index 7964c4c45a..29007807aa 100644 --- a/lisp/emacs-lisp/eldoc.el +++ b/lisp/emacs-lisp/eldoc.el @@ -5,7 +5,7 @@ ;; Author: Noah Friedman <friedman@splode.com> ;; Keywords: extensions ;; Created: 1995-10-06 -;; Version: 1.4.0 +;; Version: 1.5.0 ;; Package-Requires: ((emacs "26.3")) ;; This is a GNU ELPA :core package. Avoid functionality that is not @@ -233,7 +233,8 @@ eldoc--eval-expression-setup #'elisp-eldoc-var-docstring nil t) (add-hook 'eldoc-documentation-functions #'elisp-eldoc-funcall nil t) - (setq eldoc-documentation-strategy 'eldoc-documentation-default) + (setq-local eldoc-documentation-function nil) + (setq-local eldoc-documentation-strategy 'eldoc-documentation-default) (eldoc-mode +1)) ;;;###autoload @@ -535,8 +536,11 @@ eldoc-documentation-enthusiast (if (stringp str) (funcall callback str)) nil)))) -(define-obsolete-variable-alias 'eldoc-documentation-function - 'eldoc-documentation-strategy "eldoc-1.1.0") +(make-obsolete-variable + 'eldoc-documentation-function + "use `eldoc-documentation-strategy' instead." "eldoc-1.5.0") + +(defvar eldoc-documentation-function nil) (defcustom eldoc-documentation-strategy #'eldoc-documentation-default "How to collect and organize results of `eldoc-documentation-functions'. @@ -584,9 +588,14 @@ eldoc-documentation-strategy (function :tag "Other function")) :version "28.1") +(defun eldoc--documentation-strategy () + "Return the actual " + (or (bound-and-true-p eldoc-documentation-function) + eldoc-documentation-strategy)) + (defun eldoc--supported-p () "Non-nil if an ElDoc function is set for this buffer." - (and (not (memq eldoc-documentation-strategy '(nil ignore))) + (and (not (memq (eldoc--documentation-strategy) '(nil ignore))) (or eldoc-documentation-functions ;; The old API had major modes set `eldoc-documentation-function' ;; to provide eldoc support. It's impossible now to determine @@ -596,7 +605,8 @@ eldoc--supported-p ;; `eldoc-documentation-functions' (as in the new API). ;; But at least if it's set buffer-locally it's a good hint that ;; there's some eldoc support in the current buffer. - (local-variable-p 'eldoc-documentation-strategy)))) + (local-variable-p 'eldoc-documentation-strategy) + (local-variable-p 'eldoc-documentation-function)))) (defvar eldoc--enthusiasm-curbing-timer nil "Timer used by the `eldoc-documentation-enthusiast' strategy. @@ -703,7 +713,7 @@ eldoc--invoke-strategy (display-doc) t)))))) (let* ((eldoc--make-callback #'make-callback) - (res (funcall eldoc-documentation-strategy))) + (res (funcall (eldoc--documentation-strategy)))) ;; Observe the old and the new protocol: (cond (;; Old protocol: got string, output immediately; (stringp res) (register-doc 0 res nil) (display-doc)) ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-07-11 8:06 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <20200710004934.18557.69586@vcs0.savannah.gnu.org> [not found] ` <20200710004936.2935520A27@vcs0.savannah.gnu.org> 2020-07-10 16:09 ` master 9ade7ea: Fix Eldoc problem when loading on Emacs 26.3 Glenn Morris 2020-07-10 17:09 ` João Távora 2020-07-10 17:36 ` Noam Postavsky 2020-07-10 17:54 ` João Távora 2020-07-10 19:57 ` João Távora 2020-07-10 21:35 ` Stefan Monnier 2020-07-10 21:58 ` João Távora 2020-07-10 22:16 ` Stefan Monnier 2020-07-10 22:41 ` João Távora 2020-07-10 23:44 ` Stefan Monnier 2020-07-11 8:06 ` João Távora
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.