From: Stefan Monnier <monnier@iro.umontreal.ca>
To: Ihor Radchenko <yantar92@gmail.com>
Cc: emacs-orgmode@gnu.org
Subject: Re: Use lexical-scoping in tests
Date: Thu, 15 Sep 2022 13:29:20 -0400 [thread overview]
Message-ID: <jwvedwczhbv.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <87mtb0vhsx.fsf@localhost> (Ihor Radchenko's message of "Thu, 15 Sep 2022 22:22:22 +0800")
>> + ;; 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
next prev parent reply other threads:[~2022-09-15 17:29 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-13 0:05 Use lexical-scoping in tests Stefan Monnier
2022-09-14 12:32 ` Ihor Radchenko
2022-09-14 18:23 ` Stefan Monnier
2022-09-14 21:34 ` Stefan Monnier
2022-09-15 14:22 ` Ihor Radchenko
2022-09-15 17:29 ` Stefan Monnier [this message]
2022-09-16 3:38 ` Ihor Radchenko
2022-09-16 3:59 ` Stefan Monnier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=jwvedwczhbv.fsf-monnier+emacs@gnu.org \
--to=monnier@iro.umontreal.ca \
--cc=emacs-orgmode@gnu.org \
--cc=yantar92@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.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).