Re: Last use of defadvice in Emacs

["T.V Raman" <raman@google.com>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=gb18030, Size: 3730 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> writes:


Hi Stefan,

Could you give some tips on what  
issues one might see with defadvice and it's disrespect of lexical
bindings?

For the record, I switched emacspeak to lexical binding a couple of
years ago and fixed any/all bytecomp warnings it generated --- and I've
not hit any bugs to date. But it would be helpful to understand the
types of issues defadvice might cause with lexical binding if something
inexplicable does show up.

This next is likely not lexical binding related, but the only issue I
needed to create an emacspeak specific check was for the equivalent of
(called-interactively 'interactive) which exhibited strange behavior
with defadvice -- I created a "tighter" check that is
emacspeak-specific.


>>> The patch below replaces the last remaining use of `defadvice` in Emacs
>>> (well, except for Org where this has already been fixed upstream but
>>> we're waiting for the change to trickle down to `master`).
>>
>> Why would we want to replace defadvice with advice-add?
>> Don't all the objections to advice apply equally to both forms?
>
> Both should be avoided, indeed.  But `defadvice` is slowly being
> replaced because it cannot obey `lexical-binding` (along with a bunch
> of more minor annoyances) so it gets a few more objections.
>
>> I've spent an hour trying various combinations of eval-when-compile and
>> (boundp 'font-lock-extend-after-change-region-function), to try and get
>> the stuff compiled or ignored based on the presence/absence of that
>> variable.  Something like C's #ifdef.  I didn't manage it, and don't
>> think it's possible.  That's another C facility Emacs Lisp seems to be
>> missing.
>
> I thought the code was written specifically to perform the test at
> runtime, so that the code compiled on Emacs-21 would still skip the
> advice when run on Emacs¡Ý22 (and also so code compiled on Emacs-28 would
> still activate the advice when run on some hypothetical future Emacs
> where `font-lock-extend-after-change-region-function` has been removed).
>
> If you want to perform the test at compile time then I think something
> like the following should work.
>
>
>         Stefan
>
>
> diff --git a/lisp/progmodes/cc-mode.el b/lisp/progmodes/cc-mode.el
> index 957a0b8a7c5..05da61dbb2f 100644
> --- a/lisp/progmodes/cc-mode.el
> +++ b/lisp/progmodes/cc-mode.el
> @@ -2565,18 +2565,18 @@
>  ;; Emacs < 22 and XEmacs
>  (defmacro c-advise-fl-for-region (function)
>    (declare (debug t))
> +  (unless (boundp 'font-lock-extend-after-change-region-function)
>    `(defadvice ,function (before get-awk-region activate)
>       ;; Make sure that any string/regexp is completely font-locked.
>       (when c-buffer-is-cc-mode
>         (save-excursion
>  	 (ad-set-arg 1 c-new-END)   ; end
> -	 (ad-set-arg 0 c-new-BEG)))))	; beg
> +	   (ad-set-arg 0 c-new-BEG)))))) ; beg
>  
> -(unless (boundp 'font-lock-extend-after-change-region-function)
> -  (c-advise-fl-for-region font-lock-after-change-function)
> -  (c-advise-fl-for-region jit-lock-after-change)
> -  (c-advise-fl-for-region lazy-lock-defer-rest-after-change)
> -  (c-advise-fl-for-region lazy-lock-defer-line-after-change))
> +(c-advise-fl-for-region font-lock-after-change-function)
> +(c-advise-fl-for-region jit-lock-after-change)
> +(c-advise-fl-for-region lazy-lock-defer-rest-after-change)
> +(c-advise-fl-for-region lazy-lock-defer-line-after-change)
>  
>  ;; Connect up to `electric-indent-mode' (Emacs 24.4 and later).
>  (defun c-electric-indent-mode-hook ()
>
>

-- 

Thanks,

--Raman(I Search, I Find, I Misplace, I Research)
7©4 Id: kg:/m/0285kf1  •0Ü8