* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically [not found] ` <E1YX3rB-0005WV-PA@vcs.savannah.gnu.org> @ 2015-03-15 9:12 ` Daniel Colascione 2015-03-15 15:11 ` Artur Malabarba ` (2 more replies) 2015-03-15 15:09 ` Artur Malabarba 1 sibling, 3 replies; 32+ messages in thread From: Daniel Colascione @ 2015-03-15 9:12 UTC (permalink / raw) To: emacs-devel, Tassilo Horn [-- Attachment #1: Type: text/plain, Size: 630 bytes --] On 03/15/2015 01:25 AM, Tassilo Horn wrote: > branch: master > commit 51e7e463e93708a0e40688f91200e9e9869ec662 > Author: Tassilo Horn <tsdh@gnu.org> > Commit: Tassilo Horn <tsdh@gnu.org> > > Font-lock elisp macros/special forms dynamically Is it really a good idea to highlight *all* macros? Many are meant to be used just like functions and have the same semantics. Instead of updating a big macro regexp after load, there should be a `declare' attribute that would let specific macros (and functions) opt into being fontified specially. As it is, I'm going to have to disable this functionality locally. [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-15 9:12 ` [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically Daniel Colascione @ 2015-03-15 15:11 ` Artur Malabarba 2015-03-15 17:20 ` Drew Adams 2015-03-15 19:15 ` Stefan Monnier 2 siblings, 0 replies; 32+ messages in thread From: Artur Malabarba @ 2015-03-15 15:11 UTC (permalink / raw) To: Daniel Colascione; +Cc: Tassilo Horn, emacs-devel > Is it really a good idea to highlight *all* macros? Many are meant to be > used just like functions and have the same semantics. Instead of > updating a big macro regexp after load, there should be a `declare' > attribute that would let specific macros (and functions) opt into being > fontified specially. Maybe it should be opt-out. Even if a macro is used similarly to a function, it behaves differently, and it's good to give the user a visual notification of that. (And if the macro behaves exactly like a function it should be a function). ^ permalink raw reply [flat|nested] 32+ messages in thread
* RE: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-15 9:12 ` [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically Daniel Colascione 2015-03-15 15:11 ` Artur Malabarba @ 2015-03-15 17:20 ` Drew Adams 2015-03-15 19:15 ` Stefan Monnier 2 siblings, 0 replies; 32+ messages in thread From: Drew Adams @ 2015-03-15 17:20 UTC (permalink / raw) To: Daniel Colascione, emacs-devel, Tassilo Horn > Is it really a good idea to highlight *all* macros? Many are meant to be > used just like functions and have the same semantics. Instead of > updating a big macro regexp after load, there should be a `declare' > attribute that would let specific macros (and functions) opt into being > fontified specially. > > As it is, I'm going to have to disable this functionality locally. +1 Just what is this additional highlighting really trying to accomplish? What problem is it trying to fix? Presumably this corresponds to bug #20096, whose sole motivation was said to be: "I like the addition of if-let and when-let in subr-x.el. However, they should also be highlighted as keywords if subr-x is loaded." To that "I like", the "fix" proposed by Stefan was: "change elisp-mode's font-lock rules so they check obarray for macros", and highlight them all. As Stefan put it: "That should kill several birds with a single (big) stone." A big stone, indeed. But what are the "several birds" that need to be killed, and why? No mention. From two unhighlighted macros that one user wants highlighted, we've moved to highlighting all macros. No discussion of the why-do-this; discussion only of how-to-do-it. If you must introduce such exhuberent highlighting, perhaps you would consider throttling it via `font-lock-maximum-decoration'? (And maybe consider highlighting only those two "I like" macros?) Yes to Daniel's "let specific...opt into being fontified specially". http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20096 ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-15 9:12 ` [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically Daniel Colascione 2015-03-15 15:11 ` Artur Malabarba 2015-03-15 17:20 ` Drew Adams @ 2015-03-15 19:15 ` Stefan Monnier 2015-03-16 1:35 ` Artur Malabarba 2015-03-16 7:41 ` Tassilo Horn 2 siblings, 2 replies; 32+ messages in thread From: Stefan Monnier @ 2015-03-15 19:15 UTC (permalink / raw) To: Daniel Colascione; +Cc: Tassilo Horn, emacs-devel > Is it really a good idea to highlight *all* macros? Many are meant to be > used just like functions and have the same semantics. IMNSHO most of those are mistakes (should use compiler-macros instead). > Instead of updating a big macro regexp after load, there should be > a `declare' attribute that would let specific macros (and functions) > opt into being fontified specially. Maybe the other way around would be better: add a declaration that says "this macro faithfully mimicks the behavior of a function". > As it is, I'm going to have to disable this functionality locally. I must admit I haven't tried it out yet, so I'm not sure if I'll like the result. Stefan ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-15 19:15 ` Stefan Monnier @ 2015-03-16 1:35 ` Artur Malabarba 2015-03-16 3:07 ` Stefan Monnier 2015-03-16 7:41 ` Tassilo Horn 1 sibling, 1 reply; 32+ messages in thread From: Artur Malabarba @ 2015-03-16 1:35 UTC (permalink / raw) To: Stefan Monnier; +Cc: Daniel Colascione, emacs-devel, Tassilo Horn My initial reaction was the same as Drew, I was surprised at seing this implemented without previous discussion. However... I'm testing this change now and I like it! Of course there needs to be a way for macros do declare themselves non-special. But I think being special by default is a good choice. I also vote for enabling this by default. Disabling an unwanted feature is 100 times easier than discovering an unknown feature, for newbies and veterans alike. Finally, I'm positively surprised at how fast the update function is. I would have expected some lag, but haven't found any (even though I'm looking). 2015-03-15 16:15 GMT-03:00 Stefan Monnier <monnier@iro.umontreal.ca>: >> Is it really a good idea to highlight *all* macros? Many are meant to be >> used just like functions and have the same semantics. > > IMNSHO most of those are mistakes (should use compiler-macros instead). > >> Instead of updating a big macro regexp after load, there should be >> a `declare' attribute that would let specific macros (and functions) >> opt into being fontified specially. > > Maybe the other way around would be better: add a declaration that says > "this macro faithfully mimicks the behavior of a function". > >> As it is, I'm going to have to disable this functionality locally. > > I must admit I haven't tried it out yet, so I'm not sure if I'll like > the result. > > > Stefan > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-16 1:35 ` Artur Malabarba @ 2015-03-16 3:07 ` Stefan Monnier 2015-03-16 7:07 ` Tassilo Horn 0 siblings, 1 reply; 32+ messages in thread From: Stefan Monnier @ 2015-03-16 3:07 UTC (permalink / raw) To: Artur Malabarba; +Cc: Daniel Colascione, emacs-devel, Tassilo Horn > Finally, I'm positively surprised at how fast the update function is. > I would have expected some lag, but haven't found any (even though I'm > looking). Loading Elisp files should be fairly rare. But there might be cases where it can be done repeatedly, but if/when faced with such a situation we should be able to handle it efficiently e.g. by checking the load-history (make sure the file did include some definitions before we bother to scan symbols and rebuild the regexp) since such "run-time loading" probably won't define new functions. Stefan ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-16 3:07 ` Stefan Monnier @ 2015-03-16 7:07 ` Tassilo Horn 2015-03-16 7:10 ` Daniel Colascione 2015-03-16 12:56 ` Stefan Monnier 0 siblings, 2 replies; 32+ messages in thread From: Tassilo Horn @ 2015-03-16 7:07 UTC (permalink / raw) To: Stefan Monnier; +Cc: Daniel Colascione, Artur Malabarba, emacs-devel Stefan Monnier <monnier@IRO.UMontreal.CA> writes: >> Finally, I'm positively surprised at how fast the update function is. >> I would have expected some lag, but haven't found any (even though I'm >> looking). > > Loading Elisp files should be fairly rare. But there might be cases > where it can be done repeatedly, but if/when faced with such a situation > we should be able to handle it efficiently e.g. by checking the > load-history (make sure the file did include some definitions before we > bother to scan symbols and rebuild the regexp) since such "run-time > loading" probably won't define new functions. Like so? --8<---------------cut here---------------start------------->8--- diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el index b4f87fd..f8591aa 100644 --- a/lisp/emacs-lisp/lisp-mode.el +++ b/lisp/emacs-lisp/lisp-mode.el @@ -198,9 +198,21 @@ Return non-nil only if the old and new value are different." (concat "(" (regexp-opt elisp-macros t) "\\_>")) (not (string= old-regex lisp--el-macro-regexp)))) -(defun lisp--el-update-after-load (_file) +(defun lisp--el-update-after-load (file) "Update `lisp--el-macro-regexp' and adjust font-lock in existing buffers." - (when (lisp--el-update-macro-regexp) + (when (and + ;; Don't trigger when a file gets reloaded. + (string= file (caar load-history)) + ;; Test if the recently loaded file defined any new macros. + (let ((load-entries (cdar load-history))) + (catch 'found + (while load-entries + (when (and (consp (car load-entries)) + (eq 'defun (caar load-entries)) + (macrop (cdar load-entries))) + (throw 'found t)) + (setq load-entries (cdr load-entries))))) + (lisp--el-update-macro-regexp)) (dolist (buf (buffer-list)) (when (derived-mode-p 'emacs-lisp-mode) (font-lock-flush))))) --8<---------------cut here---------------end--------------->8--- Bye, Tassilo ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-16 7:07 ` Tassilo Horn @ 2015-03-16 7:10 ` Daniel Colascione 2015-03-16 7:54 ` Tassilo Horn 2015-03-16 12:58 ` Stefan Monnier 2015-03-16 12:56 ` Stefan Monnier 1 sibling, 2 replies; 32+ messages in thread From: Daniel Colascione @ 2015-03-16 7:10 UTC (permalink / raw) To: Stefan Monnier, Artur Malabarba, emacs-devel [-- Attachment #1: Type: text/plain, Size: 1194 bytes --] On 03/16/2015 12:07 AM, Tassilo Horn wrote: > Stefan Monnier <monnier@IRO.UMontreal.CA> writes: > >>> Finally, I'm positively surprised at how fast the update function is. >>> I would have expected some lag, but haven't found any (even though I'm >>> looking). >> >> Loading Elisp files should be fairly rare. But there might be cases >> where it can be done repeatedly, but if/when faced with such a situation >> we should be able to handle it efficiently e.g. by checking the >> load-history (make sure the file did include some definitions before we >> bother to scan symbols and rebuild the regexp) since such "run-time >> loading" probably won't define new functions. > > Like so? Instead of doing it this way, why not make a font-lock matcher that looks at *every* initial sexp atom, calls intern-soft on it, and applies a face that depends on properties of the found symbol? This way, we'd update fontification not only after load, but also after eval-defun, and it'd be easy to make a `declare'-form that provided for exceptions from the general rule. cc-mode uses a system like this for keywords, and I think it's what Stefan was talking about earlier. [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-16 7:10 ` Daniel Colascione @ 2015-03-16 7:54 ` Tassilo Horn 2015-03-16 9:36 ` Tassilo Horn 2015-03-16 12:58 ` Stefan Monnier 1 sibling, 1 reply; 32+ messages in thread From: Tassilo Horn @ 2015-03-16 7:54 UTC (permalink / raw) To: Daniel Colascione; +Cc: Stefan Monnier, Artur Malabarba, emacs-devel [-- Attachment #1: Type: text/plain, Size: 1432 bytes --] Daniel Colascione <dancol@dancol.org> writes: >>> Loading Elisp files should be fairly rare. But there might be cases >>> where it can be done repeatedly, but if/when faced with such a >>> situation we should be able to handle it efficiently e.g. by >>> checking the load-history (make sure the file did include some >>> definitions before we bother to scan symbols and rebuild the regexp) >>> since such "run-time loading" probably won't define new functions. >> >> Like so? > > Instead of doing it this way, why not make a font-lock matcher that > looks at *every* initial sexp atom, calls intern-soft on it, and > applies a face that depends on properties of the found symbol? This > way, we'd update fontification not only after load, but also after > eval-defun, and it'd be easy to make a `declare'-form that provided > for exceptions from the general rule. Also sounds good to me, and even simpler implementation-wise. So I'm happy to implement it that way if nobody comes up with a reason why that's not the right way. And we need to decide if we're opt-in, e.g., (defmacro foo () (declare font-lock-keyword) ...) makes foo highlighted as a keyword, or if we're opt-out, e.g., all macros are highlighted by default unless declared as (defmacro foo () (declare no-font-lock-keyword) ...) And I'm open to a better declaration names than the ones above. Bye, Tassilo [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 212 bytes --] ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-16 7:54 ` Tassilo Horn @ 2015-03-16 9:36 ` Tassilo Horn 0 siblings, 0 replies; 32+ messages in thread From: Tassilo Horn @ 2015-03-16 9:36 UTC (permalink / raw) To: Daniel Colascione; +Cc: Stefan Monnier, Artur Malabarba, emacs-devel [-- Attachment #1: Type: text/plain, Size: 1999 bytes --] Tassilo Horn <tsdh@gnu.org> writes: >> Instead of doing it this way, why not make a font-lock matcher that >> looks at *every* initial sexp atom, calls intern-soft on it, and >> applies a face that depends on properties of the found symbol? This >> way, we'd update fontification not only after load, but also after >> eval-defun, and it'd be easy to make a `declare'-form that provided >> for exceptions from the general rule. > > Also sounds good to me, and even simpler implementation-wise. So I'm > happy to implement it that way if nobody comes up with a reason why > that's not the right way. I've just tried that out in a local branch and it works good. I went with the opt-in variant for now (but I can change that) so that you define macros which should be highlighted like so: --8<---------------cut here---------------start------------->8--- (defmacro if-let (bindings then &rest else) (declare (indent 2) (font-lock-keyword t) ;; <=================== HERE! (debug ((&rest (symbolp form)) form body))) (when (and (<= (length bindings) 2) (not (listp (car bindings)))) ;; Adjust the single binding case (setq bindings (list bindings))) `(let* ,(internal--build-bindings bindings) (if ,(car (internal--listify (car (last bindings)))) ,then ,@else))) --8<---------------cut here---------------end--------------->8--- IMHO, that's better than the regexp variant. However, when a new macro with `font-lock-keyword' declaration gets defined (or the value is changed for some existing macro), we still need to flush existing elisp buffers. Else, the effect won't be visible there. I'm not sure from where I should do that. On a very fine-granular basis, the function handling the `font-lock-keyword' declaration could do that. But that would mean many flushes when a file with many such macros gets loaded. Alternatively, an after-load-function could do that, but that would miss changes manifested with C-c C-e. Bye, Tassilo [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 212 bytes --] ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-16 7:10 ` Daniel Colascione 2015-03-16 7:54 ` Tassilo Horn @ 2015-03-16 12:58 ` Stefan Monnier 2015-03-16 14:47 ` Tassilo Horn 1 sibling, 1 reply; 32+ messages in thread From: Stefan Monnier @ 2015-03-16 12:58 UTC (permalink / raw) To: Daniel Colascione; +Cc: Artur Malabarba, emacs-devel > Instead of doing it this way, why not make a font-lock matcher that > looks at *every* initial sexp atom, calls intern-soft on it, That would be a valid alternative implementation, of course. It's not clear it would be better, OTOH. So until proved otherwise by a concrete problem, I think we're better off staying with the current implementation. Stefan ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-16 12:58 ` Stefan Monnier @ 2015-03-16 14:47 ` Tassilo Horn 2015-03-16 17:31 ` Stefan Monnier 0 siblings, 1 reply; 32+ messages in thread From: Tassilo Horn @ 2015-03-16 14:47 UTC (permalink / raw) To: Stefan Monnier; +Cc: Daniel Colascione, Artur Malabarba, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> Instead of doing it this way, why not make a font-lock matcher that >> looks at *every* initial sexp atom, calls intern-soft on it, > > That would be a valid alternative implementation, of course. > It's not clear it would be better, OTOH. > > So until proved otherwise by a concrete problem, I think we're better > off staying with the current implementation. Here's a patch which implements your preferred opt-out variant using Daniel's suggestion. --8<---------------cut here---------------start------------->8--- diff --git a/lisp/ChangeLog b/lisp/ChangeLog index 747a1d6..78db49e 100644 --- a/lisp/ChangeLog +++ b/lisp/ChangeLog @@ -1,3 +1,15 @@ +2015-03-16 Tassilo Horn <tsdh@gnu.org> + + * emacs-lisp/byte-run.el (macro-declarations-alist): New + declaration no-font-lock-keyword. + + * emacs-lisp/lisp-mode.el (lisp--el-update-after-load) + (lisp--el-update-macro-regexp, lisp--el-macro-regexp): Delete + functions and defconst. + (lisp--el-match-keyword): Rename from lisp--el-match-macro. + (lisp-mode-variables): Remove code for updating/initializing + lisp--el-macro-regexp. + 2015-03-15 Michael Albinus <michael.albinus@gmx.de> * net/tramp-adb.el: diff --git a/lisp/emacs-lisp/byte-run.el b/lisp/emacs-lisp/byte-run.el index caa7e3d..ea70b11 100644 --- a/lisp/emacs-lisp/byte-run.el +++ b/lisp/emacs-lisp/byte-run.el @@ -151,7 +151,13 @@ This is used by `declare'.") (list 'progn :autoload-end (list 'put (list 'quote name) ''edebug-form-spec (list 'quote spec))))) - defun-declarations-alist) + (cons + (list 'no-font-lock-keyword + #'(lambda (name _args val) + (list 'progn :autoload-end + (list 'put (list 'quote name) + ''no-font-lock-keyword (list 'quote val))))) + defun-declarations-alist)) "List associating properties of macros to their macro expansion. Each element of the list takes the form (PROP FUN) where FUN is a function. For each (PROP . VALUES) in a macro's declaration, the FUN corresponding diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el index b4f87fd..8039f7a 100644 --- a/lisp/emacs-lisp/lisp-mode.el +++ b/lisp/emacs-lisp/lisp-mode.el @@ -181,32 +181,14 @@ nil))) res)) -(defconst lisp--el-macro-regexp nil - "A regular expression matching all loaded elisp macros. -Can be updated using `lisp--el-update-macro-regexp' after new -macros were defined.") - -(defun lisp--el-update-macro-regexp () - "Update `lisp--el-update-macro-regexp' from `obarray'. -Return non-nil only if the old and new value are different." - (let ((old-regex lisp--el-macro-regexp) - (elisp-macros nil)) - (mapatoms (lambda (a) - (when (or (macrop a) (special-form-p a)) - (push (symbol-name a) elisp-macros)))) - (setq lisp--el-macro-regexp - (concat "(" (regexp-opt elisp-macros t) "\\_>")) - (not (string= old-regex lisp--el-macro-regexp)))) - -(defun lisp--el-update-after-load (_file) - "Update `lisp--el-macro-regexp' and adjust font-lock in existing buffers." - (when (lisp--el-update-macro-regexp) - (dolist (buf (buffer-list)) - (when (derived-mode-p 'emacs-lisp-mode) - (font-lock-flush))))) - -(defun lisp--el-match-macro (limit) - (re-search-forward lisp--el-macro-regexp limit t)) +(defun lisp--el-match-keyword (limit) + (catch 'found + (while (re-search-forward "(\\(\\(?:\\sw\\|\\s_\\)+\\)\\_>" limit t) + (let ((sym (intern-soft (match-string 1)))) + (when (or (special-form-p sym) + (and (macrop sym) + (not (get sym 'no-font-lock-keyword)))) + (throw 'found t)))))) (pcase-let ((`(,vdefs ,tdefs @@ -362,7 +344,7 @@ Return non-nil only if the old and new value are different." `( ;; Regexp negated char group. ("\\[\\(\\^\\)" 1 font-lock-negation-char-face prepend) ;; Control structures. Common Lisp forms. - (lisp--el-match-macro . 1) + (lisp--el-match-keyword 1 font-lock-keyword-face) ;; Exit/Feature symbols as constants. (,(concat "(\\(catch\\|throw\\|featurep\\|provide\\|require\\)\\_>" "[ \t']*\\(\\(?:\\sw\\|\\s_\\)+\\)?") @@ -543,9 +525,6 @@ font-lock keywords will not be case sensitive." . lisp-font-lock-syntactic-face-function))) (setq-local prettify-symbols-alist lisp--prettify-symbols-alist) (when elisp - (unless lisp--el-macro-regexp - (lisp--el-update-macro-regexp)) - (add-hook 'after-load-functions #'lisp--el-update-after-load) (setq-local electric-pair-text-pairs (cons '(?\` . ?\') electric-pair-text-pairs))) (setq-local electric-pair-skip-whitespace 'chomp) --8<---------------cut here---------------end--------------->8--- IMHO, it's better than the regexp-based variant because there's less and simpler code, and no refresh of the regexp is needed. I also can't see any negative point like slowness (testing with our largest elisp file, vhdl-mode.el). What the patch is still missing is (1) some code to flush font-lock in existing elisp buffers to make changes to no-font-lock-keyword declarations effective (2) adding (declare (no-font-lock-keyword t)) to all function-like macros we have (e.g., push, pushnew) Bye, Tassilo ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-16 14:47 ` Tassilo Horn @ 2015-03-16 17:31 ` Stefan Monnier 2015-03-16 20:26 ` Tassilo Horn 0 siblings, 1 reply; 32+ messages in thread From: Stefan Monnier @ 2015-03-16 17:31 UTC (permalink / raw) To: Daniel Colascione; +Cc: Artur Malabarba, emacs-devel > using Daniel's suggestion. I like the "big optimized regexp" better than the "dynamic lookup via intern-soft" (should make for more efficient font-locking, tho I have no idea if that really proves faster in practice and even less of an idea if the difference would be measurable). Also, that loses the "update existing buffers after macro definition", so it's far from "clearly better" w.r.t end-user behavior (and the relative simplicity advantage will probably end up vanishing if we try to fill those kinds of differences). But it's largely a question of bikeshed color, so if you all prefer the intern-soft approach, go for it. > (2) adding (declare (no-font-lock-keyword t)) to all function-like > macros we have (e.g., push, pushnew) push and pushnew aren't function-like, so they *should* be highlighted as keywords, I think. Stefan ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-16 17:31 ` Stefan Monnier @ 2015-03-16 20:26 ` Tassilo Horn 2015-03-16 20:39 ` Daniel Colascione 0 siblings, 1 reply; 32+ messages in thread From: Tassilo Horn @ 2015-03-16 20:26 UTC (permalink / raw) To: Stefan Monnier; +Cc: Daniel Colascione, Artur Malabarba, emacs-devel Stefan Monnier <monnier@IRO.UMontreal.CA> writes: >> using Daniel's suggestion. > > I like the "big optimized regexp" better than the "dynamic lookup via > intern-soft" (should make for more efficient font-locking, tho I have > no idea if that really proves faster in practice and even less of an > idea if the difference would be measurable). I've tried it on the 18.000 LOC vhdl-mode.el. --8<---------------cut here---------------start------------->8--- ;; That's the intern-soft variant (defun font-lock-benchmark () (interactive) (message "Benchmark: %s" (benchmark-run-compiled 30 (goto-char (point-min)) (while (lisp--el-match-keyword (point-max)))))) (defconst macro-rx (concat "(" (let (macros) (mapatoms (lambda (a) (when (or (special-form-p a) (macrop a)) (push (symbol-name a) macros)))) (regexp-opt macros t)) "\\_>")) ;; That's the optimized-regexp variant (defun font-lock-benchmark2 () (interactive) (message "Benchmark: %s" (benchmark-run-compiled 30 (goto-char (point-min)) (while (re-search-forward macro-rx nil t) (match-string 1))))) --8<---------------cut here---------------end--------------->8--- Indeed, the variant with the optimized regexp is more than twice as fast as the intern-soft approach. But both variants are so fast that there's no observable difference. > Also, that loses the "update existing buffers after macro definition", > so it's far from "clearly better" w.r.t end-user behavior (and the > relative simplicity advantage will probably end up vanishing if we try > to fill those kinds of differences). Yes, that's what I've said. The reason why I haven't implemented it yet is that I'm not sure where to add the relevant code. With the intern-soft approach, we can basically adjust fontification as soon as a new macro is defined or evaluated using C-c C-e because it's cheap (in contrast to updating existing buffers only when a file is loaded). But of course if a file defines 100 macros, I don't want 100 updates of existing elisp buffers. Any suggestions how/where to update existing buffers? I can imagine that `defmacro' starts or resets some idle timer which updates existing buffers. But having a byte-run -> lisp-mode dependency just seems wrong. > But it's largely a question of bikeshed color, so if you all prefer the > intern-soft approach, go for it. Currently it's mostly Daniel vs. you, so let's wait for some more opinions. >> (2) adding (declare (no-font-lock-keyword t)) to all function-like >> macros we have (e.g., push, pushnew) > > push and pushnew aren't function-like, so they *should* be highlighted > as keywords, I think. I guess they are not function-like because they deal with generalized variables, right? Bye, Tassilo ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-16 20:26 ` Tassilo Horn @ 2015-03-16 20:39 ` Daniel Colascione 2015-03-16 20:56 ` Stefan Monnier 0 siblings, 1 reply; 32+ messages in thread From: Daniel Colascione @ 2015-03-16 20:39 UTC (permalink / raw) To: Stefan Monnier, Artur Malabarba, emacs-devel [-- Attachment #1: Type: text/plain, Size: 3696 bytes --] On 03/16/2015 01:26 PM, Tassilo Horn wrote: > Stefan Monnier <monnier@IRO.UMontreal.CA> writes: > >>> using Daniel's suggestion. >> >> I like the "big optimized regexp" better than the "dynamic lookup via >> intern-soft" (should make for more efficient font-locking, tho I have >> no idea if that really proves faster in practice and even less of an >> idea if the difference would be measurable). > > I've tried it on the 18.000 LOC vhdl-mode.el. > > --8<---------------cut here---------------start------------->8--- > ;; That's the intern-soft variant > (defun font-lock-benchmark () > (interactive) > (message "Benchmark: %s" > (benchmark-run-compiled 30 > (goto-char (point-min)) > (while (lisp--el-match-keyword (point-max)))))) > > (defconst macro-rx > (concat "(" (let (macros) > (mapatoms (lambda (a) > (when (or (special-form-p a) > (macrop a)) > (push (symbol-name a) macros)))) > (regexp-opt macros t)) > "\\_>")) > > ;; That's the optimized-regexp variant > (defun font-lock-benchmark2 () > (interactive) > (message "Benchmark: %s" > (benchmark-run-compiled 30 > (goto-char (point-min)) > (while (re-search-forward macro-rx nil t) > (match-string 1))))) > --8<---------------cut here---------------end--------------->8--- > > Indeed, the variant with the optimized regexp is more than twice as fast > as the intern-soft approach. But both variants are so fast that there's > no observable difference. > >> Also, that loses the "update existing buffers after macro definition", >> so it's far from "clearly better" w.r.t end-user behavior (and the >> relative simplicity advantage will probably end up vanishing if we try >> to fill those kinds of differences). > > Yes, that's what I've said. The reason why I haven't implemented it yet > is that I'm not sure where to add the relevant code. With the > intern-soft approach, we can basically adjust fontification as soon as a > new macro is defined or evaluated using C-c C-e because it's cheap (in > contrast to updating existing buffers only when a file is loaded). But > of course if a file defines 100 macros, I don't want 100 updates of > existing elisp buffers. > > Any suggestions how/where to update existing buffers? > > I can imagine that `defmacro' starts or resets some idle timer which > updates existing buffers. But having a byte-run -> lisp-mode dependency > just seems wrong. Isn't font-lock-flush supposed to be cheap? Running it a thousand times on byte-run.el only takes 3ms. If that's too expensive, we can ordinarily run font-lock-flush immediately, but during load, bind a variable that defers the call until it's unbound. Maybe something like this around load? (defmacro with-deferred-emacs-lisp-fontification-updates (&rest body) `(let ((inhibit-emacs-lisp-fontification-update t)) ,@body (emacs-lisp-font-lock-flush-all-buffers)) > >> But it's largely a question of bikeshed color, so if you all prefer the >> intern-soft approach, go for it. > > Currently it's mostly Daniel vs. you, so let's wait for some more > opinions. > >>> (2) adding (declare (no-font-lock-keyword t)) to all function-like >>> macros we have (e.g., push, pushnew) >> >> push and pushnew aren't function-like, so they *should* be highlighted >> as keywords, I think. > > I guess they are not function-like because they deal with generalized > variables, right? Right. I'm okay with either opt-in or opt-out behavior. Maybe we can use edebug specs to detect function-like macros in the absence of other information? [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-16 20:39 ` Daniel Colascione @ 2015-03-16 20:56 ` Stefan Monnier 2015-03-17 9:36 ` Tassilo Horn 0 siblings, 1 reply; 32+ messages in thread From: Stefan Monnier @ 2015-03-16 20:56 UTC (permalink / raw) To: Daniel Colascione; +Cc: Artur Malabarba, emacs-devel > > Any suggestions how/where to update existing buffers? My suggestion would be to do it from after-load-functions, i.e. using the exact same code as the code used for the alternative implementation ;-) > Isn't font-lock-flush supposed to be cheap? If you use jit-lock-mode, yes. If not, no. >>> push and pushnew aren't function-like, so they *should* be highlighted >>> as keywords, I think. >> I guess they are not function-like because they deal with generalized >> variables, right? Right: (push x y) is not the same as (let ((z y)) (push x z)). Stefan ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-16 20:56 ` Stefan Monnier @ 2015-03-17 9:36 ` Tassilo Horn 2015-03-17 10:17 ` Artur Malabarba 2015-03-17 16:34 ` Stefan Monnier 0 siblings, 2 replies; 32+ messages in thread From: Tassilo Horn @ 2015-03-17 9:36 UTC (permalink / raw) To: emacs-devel [-- Attachment #1: Type: text/plain, Size: 1620 bytes --] Stefan Monnier <monnier@IRO.UMontreal.CA> writes: >> > Any suggestions how/where to update existing buffers? > > My suggestion would be to do it from after-load-functions, i.e. using > the exact same code as the code used for the alternative > implementation ;-) Yes, that's what I'm doing now. In addition, `defmacro' does update as well (in case of a new macro or a redefinition with different 'no-font-lock-keyword declaration). So with that, you get instant fontification updates no matter if you C-x C-e a macro definition or load a file containing macro definitions. There's at most one update per C-x C-e on a defmacro form, and exactly one update per file-load. And updates are cheaper since no regexp has to be computed. I've measured it, and compared to `font-lock-flush' (with `jit-lock-mode' enabled) collecting all macros and builting a regexp using `regexp-opt' is about 80 times more expensive. >> Isn't font-lock-flush supposed to be cheap? > > If you use jit-lock-mode, yes. If not, no. Using it is the default. Is there a good reason a user might have disabled it for elisp buffers? If no one objects, I'm going to install the patch below anytime soon when the master branch bootstraps again without Eager macro-expansion failure: (void-function cl-every) Of course, then we still need to handle the function-like macros. Daniel's suggestion of using the `debug' declaration for that doesn't work (or I don't get it). At least, having a debug declaration doesn't imply being "non-function-like", and neither does having no debug declaration imply being "function-like". Bye, Tassilo [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Improve-dynamic-elisp-keyword-font-locking.patch --] [-- Type: text/x-diff, Size: 6831 bytes --] From 2724f2899a04291e3b4ce5b6f8ddd186165ae40d Mon Sep 17 00:00:00 2001 From: Tassilo Horn <tsdh@gnu.org> Date: Mon, 16 Mar 2015 10:25:14 +0100 Subject: [PATCH] Improve dynamic elisp keyword font-locking * emacs-lisp/byte-run.el (macro-declarations-alist): New declaration no-font-lock-keyword. (defmacro): Flush font-lock in existing elisp buffers. * emacs-lisp/lisp-mode.el (lisp--el-update-after-load) (lisp--el-update-macro-regexp, lisp--el-macro-regexp): Delete functions and defconst. (lisp--el-match-keyword): Rename from lisp--el-match-macro. (lisp--el-font-lock-flush-elisp-buffers): New function. (lisp-mode-variables): Remove code for updating lisp--el-macro-regexp, and add lisp--el-font-lock-flush-elisp-buffers to after-load-functions. --- lisp/ChangeLog | 15 ++++++++++++++ lisp/emacs-lisp/byte-run.el | 28 ++++++++++++++++++++----- lisp/emacs-lisp/lisp-mode.el | 49 ++++++++++++++++++-------------------------- 3 files changed, 58 insertions(+), 34 deletions(-) diff --git a/lisp/ChangeLog b/lisp/ChangeLog index cbd1bce..88c3c01 100644 --- a/lisp/ChangeLog +++ b/lisp/ChangeLog @@ -1,3 +1,18 @@ +2015-03-17 Tassilo Horn <tsdh@gnu.org> + + * emacs-lisp/byte-run.el (macro-declarations-alist): New + declaration no-font-lock-keyword. + (defmacro): Flush font-lock in existing elisp buffers. + + * emacs-lisp/lisp-mode.el (lisp--el-update-after-load) + (lisp--el-update-macro-regexp, lisp--el-macro-regexp): Delete + functions and defconst. + (lisp--el-match-keyword): Rename from lisp--el-match-macro. + (lisp--el-font-lock-flush-elisp-buffers): New function. + (lisp-mode-variables): Remove code for updating + lisp--el-macro-regexp, and add + lisp--el-font-lock-flush-elisp-buffers to after-load-functions. + 2015-03-16 Alan Mackenzie <acm@muc.de> Edebug: Allow "S" to work during trace mode. Fixes debbugs #20074. diff --git a/lisp/emacs-lisp/byte-run.el b/lisp/emacs-lisp/byte-run.el index caa7e3d..01c1af7 100644 --- a/lisp/emacs-lisp/byte-run.el +++ b/lisp/emacs-lisp/byte-run.el @@ -147,11 +147,16 @@ This is used by `declare'.") (defvar macro-declarations-alist (cons (list 'debug - #'(lambda (name _args spec) - (list 'progn :autoload-end - (list 'put (list 'quote name) - ''edebug-form-spec (list 'quote spec))))) - defun-declarations-alist) + #'(lambda (name _args spec) + (list 'progn :autoload-end + (list 'put (list 'quote name) + ''edebug-form-spec (list 'quote spec))))) + (cons + (list 'no-font-lock-keyword + #'(lambda (name _args val) + (list 'function-put (list 'quote name) + ''no-font-lock-keyword (list 'quote val)))) + defun-declarations-alist)) "List associating properties of macros to their macro expansion. Each element of the list takes the form (PROP FUN) where FUN is a function. For each (PROP . VALUES) in a macro's declaration, the FUN corresponding @@ -201,6 +206,19 @@ The return value is undefined. (message "Warning: Unknown macro property %S in %S" (car x) name)))) decls))) + ;; Refresh font-lock if this is a new macro, or it is an + ;; existing macro whose 'no-font-lock-keyword declaration + ;; has changed. + (if (and + ;; During bootstrap, subr.el isn't loaded, but then we + ;; don't need to refresh anyway. + (fboundp 'macrop) + (fboundp 'lisp--el-font-lock-flush-elisp-buffers) + (macrop name) + (member `(function-put ',name 'no-font-lock-keyword + ',(get name 'no-font-lock-keyword)) + declarations)) + (lisp--el-font-lock-flush-elisp-buffers)) (if declarations (cons 'prog1 (cons def declarations)) def)))))) diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el index b4f87fd..6b30773 100644 --- a/lisp/emacs-lisp/lisp-mode.el +++ b/lisp/emacs-lisp/lisp-mode.el @@ -181,32 +181,25 @@ nil))) res)) -(defconst lisp--el-macro-regexp nil - "A regular expression matching all loaded elisp macros. -Can be updated using `lisp--el-update-macro-regexp' after new -macros were defined.") - -(defun lisp--el-update-macro-regexp () - "Update `lisp--el-update-macro-regexp' from `obarray'. -Return non-nil only if the old and new value are different." - (let ((old-regex lisp--el-macro-regexp) - (elisp-macros nil)) - (mapatoms (lambda (a) - (when (or (macrop a) (special-form-p a)) - (push (symbol-name a) elisp-macros)))) - (setq lisp--el-macro-regexp - (concat "(" (regexp-opt elisp-macros t) "\\_>")) - (not (string= old-regex lisp--el-macro-regexp)))) - -(defun lisp--el-update-after-load (_file) - "Update `lisp--el-macro-regexp' and adjust font-lock in existing buffers." - (when (lisp--el-update-macro-regexp) +(defun lisp--el-match-keyword (limit) + (catch 'found + (while (re-search-forward "(\\(\\(?:\\sw\\|\\s_\\)+\\)\\_>" limit t) + (let ((sym (intern-soft (match-string 1)))) + (when (or (special-form-p sym) + (and (macrop sym) + (not (get sym 'no-font-lock-keyword)))) + (throw 'found t)))))) + +(defun lisp--el-font-lock-flush-elisp-buffers (&optional file) + ;; Don't flush during load unless called from after-load-functions. + ;; In that case, FILE is non-nil. It's somehow strange that + ;; load-in-progress is t when an after-load-function is called since + ;; that should run *after* the load... + (when (or (not load-in-progress) file) (dolist (buf (buffer-list)) - (when (derived-mode-p 'emacs-lisp-mode) - (font-lock-flush))))) - -(defun lisp--el-match-macro (limit) - (re-search-forward lisp--el-macro-regexp limit t)) + (with-current-buffer buf + (when (derived-mode-p 'emacs-lisp-mode) + (font-lock-flush)))))) (pcase-let ((`(,vdefs ,tdefs @@ -362,7 +355,7 @@ Return non-nil only if the old and new value are different." `( ;; Regexp negated char group. ("\\[\\(\\^\\)" 1 font-lock-negation-char-face prepend) ;; Control structures. Common Lisp forms. - (lisp--el-match-macro . 1) + (lisp--el-match-keyword . 1) ;; Exit/Feature symbols as constants. (,(concat "(\\(catch\\|throw\\|featurep\\|provide\\|require\\)\\_>" "[ \t']*\\(\\(?:\\sw\\|\\s_\\)+\\)?") @@ -543,9 +536,7 @@ font-lock keywords will not be case sensitive." . lisp-font-lock-syntactic-face-function))) (setq-local prettify-symbols-alist lisp--prettify-symbols-alist) (when elisp - (unless lisp--el-macro-regexp - (lisp--el-update-macro-regexp)) - (add-hook 'after-load-functions #'lisp--el-update-after-load) + (add-hook 'after-load-functions #'lisp--el-font-lock-flush-elisp-buffers) (setq-local electric-pair-text-pairs (cons '(?\` . ?\') electric-pair-text-pairs))) (setq-local electric-pair-skip-whitespace 'chomp) -- 2.3.3 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-17 9:36 ` Tassilo Horn @ 2015-03-17 10:17 ` Artur Malabarba 2015-03-17 16:34 ` Stefan Monnier 1 sibling, 0 replies; 32+ messages in thread From: Artur Malabarba @ 2015-03-17 10:17 UTC (permalink / raw) To: emacs-devel, Daniel Colascione [-- Attachment #1: Type: text/plain, Size: 519 bytes --] > Of course, then we still need to handle the function-like macros. > Daniel's suggestion of using the `debug' declaration for that doesn't > work (or I don't get it). At least, having a debug declaration doesn't > imply being "non-function-like", and neither does having no debug > declaration imply being "function-like". Yes, I had thought about suggesting that but it wouldn't work. There are plenty of very macro-y macros whose arguments are function-like for debug purpose. See, for instance, with-temp-buffer. [-- Attachment #2: Type: text/html, Size: 641 bytes --] ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-17 9:36 ` Tassilo Horn 2015-03-17 10:17 ` Artur Malabarba @ 2015-03-17 16:34 ` Stefan Monnier 2015-03-17 16:47 ` Nicolas Richard 2015-03-18 7:17 ` Tassilo Horn 1 sibling, 2 replies; 32+ messages in thread From: Stefan Monnier @ 2015-03-17 16:34 UTC (permalink / raw) To: Daniel Colascione; +Cc: Artur Malabarba, emacs-devel >>> Isn't font-lock-flush supposed to be cheap? >> If you use jit-lock-mode, yes. If not, no. > Using it is the default. Is there a good reason a user might have > disabled it for elisp buffers? There ideally shouldn't be a good reason, no. It can be very useful while debugging font-lock-keywords, and things like that, but for "normal" use, jit-lock-mode should always be used in Elisp buffers. > If no one objects, I'm going to install the patch below anytime soon > when the master branch bootstraps again without > Eager macro-expansion failure: (void-function cl-every) Isn't this fixed already (at least, the cl-every I introduced has disappeared as a side-effect of a subsequent change)? Stefan ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-17 16:34 ` Stefan Monnier @ 2015-03-17 16:47 ` Nicolas Richard 2015-03-18 7:17 ` Tassilo Horn 1 sibling, 0 replies; 32+ messages in thread From: Nicolas Richard @ 2015-03-17 16:47 UTC (permalink / raw) To: Stefan Monnier; +Cc: Daniel Colascione, Artur Malabarba, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: > Isn't this fixed already (at least, the cl-every I introduced has > disappeared as a side-effect of a subsequent change)? I still see it : $ git --no-pager grep cl-every savannah/master:lisp/emacs-lisp/cl-macs.el savannah/master:lisp/emacs-lisp/cl-macs.el: (cl-every (lambda (x) (null (cadr x))) (cdr cl--bind-defs))) $ git rev-parse savannah/master 508049aae95c42a3e6fe989ff403bf7cb6f88433 -- Nico ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-17 16:34 ` Stefan Monnier 2015-03-17 16:47 ` Nicolas Richard @ 2015-03-18 7:17 ` Tassilo Horn 2015-03-18 9:10 ` Artur Malabarba 2015-03-18 13:13 ` Stefan Monnier 1 sibling, 2 replies; 32+ messages in thread From: Tassilo Horn @ 2015-03-18 7:17 UTC (permalink / raw) To: Stefan Monnier; +Cc: Daniel Colascione, Artur Malabarba, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >>>> Isn't font-lock-flush supposed to be cheap? >>> If you use jit-lock-mode, yes. If not, no. >> Using it is the default. Is there a good reason a user might have >> disabled it for elisp buffers? > > There ideally shouldn't be a good reason, no. It can be very useful > while debugging font-lock-keywords, and things like that, but for > "normal" use, jit-lock-mode should always be used in Elisp buffers. Ok. I just tried it anyway, and with jit-lock disabled, `font-lock-flush' does nothing, e.g., the buffer portion you have already scrolled through is still fontified as it has been before but the yet "unknown" portions are and stay in the default face when scrolling to them. So it seems like in non-jit-locked buffers we'd actually need to call f-l-flush followed by f-l-ensure. But OTOH, when someone turns off jit-lock to debug fontification, she might not like refreshed from the outside so that behavior could be seen as justified there... >> If no one objects, I'm going to install the patch below anytime soon >> when the master branch bootstraps again without >> Eager macro-expansion failure: (void-function cl-every) > > Isn't this fixed already (at least, the cl-every I introduced has > disappeared as a side-effect of a subsequent change)? Yes, yes, its fixed. I've committed and pushed my last patch (with some fixes since the posted version) as 9fdc166. If no one comes up with a (semi-)automated way of identifying function-like macros, I'll go check the ~1300 macros in emacs manually. For that, do we have clear criteria when we consider a macro function-like? Here's my list so far: - function-like: + has no arguments + just there to save a funcall (ad-get-advice-info-macro, shadow-cluster-name) Shouldn't those be defsubsts instead of macros? - not function-like: + is a control structure (when, when-let, cl-loop,...) + modifies places (push, pushnew, setf, ...) + defines something (def*, define-*) + sets up some environment (with-*, save-*) + error handling (ignore-errors, condition-case) + anaphora Bye, Tassilo ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-18 7:17 ` Tassilo Horn @ 2015-03-18 9:10 ` Artur Malabarba 2015-03-18 9:20 ` Dmitry Gutov 2015-03-18 13:13 ` Stefan Monnier 1 sibling, 1 reply; 32+ messages in thread From: Artur Malabarba @ 2015-03-18 9:10 UTC (permalink / raw) To: Stefan Monnier, Daniel Colascione, Artur Malabarba, emacs-devel > + just there to save a funcall (ad-get-advice-info-macro, > shadow-cluster-name) > Shouldn't those be defsubsts instead of macros? Yes, I believe they should. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-18 9:10 ` Artur Malabarba @ 2015-03-18 9:20 ` Dmitry Gutov 0 siblings, 0 replies; 32+ messages in thread From: Dmitry Gutov @ 2015-03-18 9:20 UTC (permalink / raw) To: bruce.connor.am, Stefan Monnier, Daniel Colascione, emacs-devel On 03/18/2015 11:10 AM, Artur Malabarba wrote: >> + just there to save a funcall (ad-get-advice-info-macro, >> shadow-cluster-name) >> Shouldn't those be defsubsts instead of macros? Or plain functions, with compiler-macro declarations? ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-18 7:17 ` Tassilo Horn 2015-03-18 9:10 ` Artur Malabarba @ 2015-03-18 13:13 ` Stefan Monnier 2015-03-18 16:12 ` Tassilo Horn 1 sibling, 1 reply; 32+ messages in thread From: Stefan Monnier @ 2015-03-18 13:13 UTC (permalink / raw) To: Daniel Colascione; +Cc: Artur Malabarba, emacs-devel > Ok. I just tried it anyway, and with jit-lock disabled, > `font-lock-flush' does nothing, e.g., the buffer portion you have I think you've just found a bug. How did you disable jit-lock? Can you try "emacs -Q --eval '(setq font-lock-support-mode nil)'" and retry your test. > If no one comes up with a (semi-)automated way of identifying > function-like macros, I'll go check the ~1300 macros in emacs manually. We could also do it lazily: wait for people to complain and fix those that come up. > + just there to save a funcall (ad-get-advice-info-macro, > shadow-cluster-name) > Shouldn't those be defsubsts instead of macros? Yes, but sometimes having them as macros is better (e.g., the code should be a bit faster than with a defsubst; also macros are expanded by gv.el so defining them as macros lets you use them as places in `setf' and `push' as well. The new `define-inline' does address both of those problems, OTOH). Stefan ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-18 13:13 ` Stefan Monnier @ 2015-03-18 16:12 ` Tassilo Horn 2015-03-18 16:44 ` Stefan Monnier 0 siblings, 1 reply; 32+ messages in thread From: Tassilo Horn @ 2015-03-18 16:12 UTC (permalink / raw) To: Stefan Monnier; +Cc: Daniel Colascione, Artur Malabarba, emacs-devel Stefan Monnier <monnier@IRO.UMontreal.CA> writes: >> Ok. I just tried it anyway, and with jit-lock disabled, >> `font-lock-flush' does nothing, e.g., the buffer portion you have > > I think you've just found a bug. How did you disable jit-lock? Finding vhdl-mode.el (because that's large) and then doing M-: (jit-lock-mode nil). > Can you try "emacs -Q --eval '(setq font-lock-support-mode nil)'" and > retry your test. Yes. So I did that and found vhdl-mode.el again. Nothing is fontified. (font-lock-flush) doesn't change that, but (font-lock-ensure) fontifies the complete buffer. >> If no one comes up with a (semi-)automated way of identifying >> function-like macros, I'll go check the ~1300 macros in emacs >> manually. > > We could also do it lazily: wait for people to complain and fix those > that come up. That's a good plan. :-) So Daniel and Drew, feel free to complain about concrete macros. Or just add a (declare (no-font-lock-keyword t)) to them yourself. >> + just there to save a funcall (ad-get-advice-info-macro, >> shadow-cluster-name) >> Shouldn't those be defsubsts instead of macros? > > Yes, but sometimes having them as macros is better (e.g., the code > should be a bit faster than with a defsubst; also macros are expanded > by gv.el so defining them as macros lets you use them as places in > `setf' and `push' as well. The new `define-inline' does address both > of those problems, OTOH). Oh, cool. Is that going to be an alternative to `defsubst' or is the plan to make it `defsubst' at some point in time? (At least from a very brief look, it seems to provide the same interface for the same use-case.) Bye, Tassilo ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-18 16:12 ` Tassilo Horn @ 2015-03-18 16:44 ` Stefan Monnier 0 siblings, 0 replies; 32+ messages in thread From: Stefan Monnier @ 2015-03-18 16:44 UTC (permalink / raw) To: Daniel Colascione; +Cc: Artur Malabarba, emacs-devel > Finding vhdl-mode.el (because that's large) and then doing M-: > (jit-lock-mode nil). jit-lock-mode used to be a proper minor mode, but it's not any more and calling this `jit-lock-mode function is basically a bad idea. >> Can you try "emacs -Q --eval '(setq font-lock-support-mode nil)'" and >> retry your test. > Yes. So I did that and found vhdl-mode.el again. Nothing is fontified. That's because vhdl-mode.el is larger than font-lock-maximum-size. > Oh, cool. Is that going to be an alternative to `defsubst' It's an alternative. > or is the plan to make it `defsubst' at some point in time? It's not compatible. Stefan ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-16 7:07 ` Tassilo Horn 2015-03-16 7:10 ` Daniel Colascione @ 2015-03-16 12:56 ` Stefan Monnier 1 sibling, 0 replies; 32+ messages in thread From: Stefan Monnier @ 2015-03-16 12:56 UTC (permalink / raw) To: Artur Malabarba; +Cc: Daniel Colascione, emacs-devel > Like so? For example, yes. > + ;; Don't trigger when a file gets reloaded. > + (string= file (caar load-history)) Tho I wouldn't do that. By "repeatedly" I didn't mean literally the same file. I was thinking more of cases like calling `load' on all the ~/.emacs.d/elpa/**/*-pkgs.el files (which we don't do any more, BTW). Stefan ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-15 19:15 ` Stefan Monnier 2015-03-16 1:35 ` Artur Malabarba @ 2015-03-16 7:41 ` Tassilo Horn 2015-03-16 12:59 ` Stefan Monnier 1 sibling, 1 reply; 32+ messages in thread From: Tassilo Horn @ 2015-03-16 7:41 UTC (permalink / raw) To: Stefan Monnier; +Cc: Daniel Colascione, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> Instead of updating a big macro regexp after load, there should be a >> `declare' attribute that would let specific macros (and functions) >> opt into being fontified specially. > > Maybe the other way around would be better: add a declaration that > says "this macro faithfully mimicks the behavior of a function". Both opt-in and opt-out sound reasonable to me, and I don't have a preference. So just tell me what you prefer. AFAICS, I'd need to add a new `(no-)font-lock-keyword' entry to `macro-declarations-alist' and then check (get macro '(no-)font-lock-keyword) in the code building the regex, right? Or in case of opt-in, (declare 'font-lock-keyword) could add the symbol to a list of macros to font-lock as keywords so that we don't need to check all symbols in `obarray'. The name (no-)font-lock-keyword is open to discussion, of course. Also, I'd like to rename the new lisp--el-*-macro-* symbols to lisp--el-*-keyword-* since they deal with both macros and special forms, i.e., they deal with stuff font-locked with keyword face. Bye, Tassilo ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-16 7:41 ` Tassilo Horn @ 2015-03-16 12:59 ` Stefan Monnier 2015-03-16 14:23 ` Artur Malabarba 0 siblings, 1 reply; 32+ messages in thread From: Stefan Monnier @ 2015-03-16 12:59 UTC (permalink / raw) To: Daniel Colascione; +Cc: emacs-devel > Both opt-in and opt-out sound reasonable to me, and I don't have a > preference. So just tell me what you prefer. I much prefer opt-out. Stefan ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-16 12:59 ` Stefan Monnier @ 2015-03-16 14:23 ` Artur Malabarba 0 siblings, 0 replies; 32+ messages in thread From: Artur Malabarba @ 2015-03-16 14:23 UTC (permalink / raw) To: Stefan Monnier; +Cc: Daniel Colascione, emacs-devel [-- Attachment #1: Type: text/plain, Size: 152 bytes --] > > Both opt-in and opt-out sound reasonable to me, and I don't have a > > preference. So just tell me what you prefer. > > I much prefer opt-out. +1 [-- Attachment #2: Type: text/html, Size: 225 bytes --] ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: master 51e7e46: Font-lock elisp macros/special forms dynamically [not found] ` <E1YX3rB-0005WV-PA@vcs.savannah.gnu.org> 2015-03-15 9:12 ` [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically Daniel Colascione @ 2015-03-15 15:09 ` Artur Malabarba 2015-03-16 6:40 ` [Emacs-diffs] " Tassilo Horn 1 sibling, 1 reply; 32+ messages in thread From: Artur Malabarba @ 2015-03-15 15:09 UTC (permalink / raw) To: emacs-devel, Tassilo Horn; +Cc: emacs-diffs > + (unless lisp--el-macro-regexp > + (lisp--el-update-macro-regexp)) > + (add-hook 'after-load-functions #'lisp--el-update-after-load) Maybe I'm reading this wrong, but is this being automatically added to `after-load-functions'? If so, I request we don't do that by default. I haven't tested it yet, but I'm sure mapping over all atoms and flushing all elisp buffers is bound to cause noticeable delays whenever a file is loaded. It could be turned on with a global minor mode. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically 2015-03-15 15:09 ` Artur Malabarba @ 2015-03-16 6:40 ` Tassilo Horn 0 siblings, 0 replies; 32+ messages in thread From: Tassilo Horn @ 2015-03-16 6:40 UTC (permalink / raw) To: Artur Malabarba; +Cc: emacs-diffs, emacs-devel Artur Malabarba <bruce.connor.am@gmail.com> writes: >> + (unless lisp--el-macro-regexp >> + (lisp--el-update-macro-regexp)) >> + (add-hook 'after-load-functions #'lisp--el-update-after-load) > > Maybe I'm reading this wrong, but is this being automatically added to > `after-load-functions'? Yes. > If so, I request we don't do that by default. I haven't tested it yet, > but I'm sure mapping over all atoms and flushing all elisp buffers is > bound to cause noticeable delays whenever a file is loaded. It only flushes elisp buffers in case the regexp actually changed. So if a file got loaded which doesn't define any new macros, no re-fontification will happen. Bye, Tassilo ^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2015-03-18 16:44 UTC | newest] Thread overview: 32+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <20150315082509.21193.18465@vcs.savannah.gnu.org> [not found] ` <E1YX3rB-0005WV-PA@vcs.savannah.gnu.org> 2015-03-15 9:12 ` [Emacs-diffs] master 51e7e46: Font-lock elisp macros/special forms dynamically Daniel Colascione 2015-03-15 15:11 ` Artur Malabarba 2015-03-15 17:20 ` Drew Adams 2015-03-15 19:15 ` Stefan Monnier 2015-03-16 1:35 ` Artur Malabarba 2015-03-16 3:07 ` Stefan Monnier 2015-03-16 7:07 ` Tassilo Horn 2015-03-16 7:10 ` Daniel Colascione 2015-03-16 7:54 ` Tassilo Horn 2015-03-16 9:36 ` Tassilo Horn 2015-03-16 12:58 ` Stefan Monnier 2015-03-16 14:47 ` Tassilo Horn 2015-03-16 17:31 ` Stefan Monnier 2015-03-16 20:26 ` Tassilo Horn 2015-03-16 20:39 ` Daniel Colascione 2015-03-16 20:56 ` Stefan Monnier 2015-03-17 9:36 ` Tassilo Horn 2015-03-17 10:17 ` Artur Malabarba 2015-03-17 16:34 ` Stefan Monnier 2015-03-17 16:47 ` Nicolas Richard 2015-03-18 7:17 ` Tassilo Horn 2015-03-18 9:10 ` Artur Malabarba 2015-03-18 9:20 ` Dmitry Gutov 2015-03-18 13:13 ` Stefan Monnier 2015-03-18 16:12 ` Tassilo Horn 2015-03-18 16:44 ` Stefan Monnier 2015-03-16 12:56 ` Stefan Monnier 2015-03-16 7:41 ` Tassilo Horn 2015-03-16 12:59 ` Stefan Monnier 2015-03-16 14:23 ` Artur Malabarba 2015-03-15 15:09 ` Artur Malabarba 2015-03-16 6:40 ` [Emacs-diffs] " Tassilo Horn
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).