emacs-orgmode
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Use lexical-scoping in tests


From: Stefan Monnier
Subject: Re: Use lexical-scoping in tests
Date: Thu, 15 Sep 2022 13:29:20 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux)

>> +            ;; FIXME: `s' is a symbol, so (car-safe s) is always nil.
>> +            ;;(when (eq 'autoload (car-safe s))
>> +            ;;  (unintern s obarray))
>> +            ))))
>
> If I understand correctly, the intended version of this code is supposed
> to be
>
>   (when (autoloadp (symbol-function s))
>     (unintern s obarray))
>
> the idea being "unloading" all the built-in org-related staff.
> However, make test will be failing then with byte-compiler error.
> I feel that the idea of the code is reasonable, but some detail of how
> autoloads work in Emacs is missed.

I don't see why you'd need to remove the existing autoloads: they don't
specify which directory the file will come from, so if the file still
exists in the new Org, the (old) autoload will load from the new Org,
as needed.

I mean it's OK to remove those autoloads, but really, those are usually
harmless and they are the least of your problems.  I think the things
we'd want/need to "remove" are those things which *aren't* autoloads.

Also `unintern` is a fairly powerful operation which can come with
undesirable side-effects, so I'd rather replace it with something less
risky like `fmakunbound`.

>> +       ;; FIXME: For the rare cases where we do need to mess with windows,
>> +           ;; we should let `body' take care of displaying this buffer!
>>         (setq buffer (find-file file))
>
> Could you please elaborate about this fixme?

`find-file` displays the buffer in a window.  In most uses of this code
we don't care whether the buffer is displayed or not, so we should
probably use `find-file-noselect` instead.
[ As a rule of thumb, most uses of `find-file` (and `switch-to-buffer`)
  in ELisp code are the result of a misunderstanding from the coder who
  just uses the commands he's familiar with as a user.  But in Elisp,
  you generally want to use `find-file-noselect` (and `set-buffer`, or
  maybe `pop-to-buffer`) instead.  ]

I didn't make the change, tho, because some parts of the tests do care
about which buffer is displayed in which window, apparently, so it would
take more work.


        Stefan




reply via email to

[Prev in Thread] Current Thread [Next in Thread]