GNU bug report logs - #76372
31.0.50; False () in ert-font-lock macro signatures

Previous Next

Package: emacs;

Reported by: "Basil L. Contovounesios" <basil <at> contovou.net>

Date: Mon, 17 Feb 2025 20:38:02 UTC

Severity: normal

Tags: patch

Found in versions 31.0.50, 30.0.93

Fixed in version 31.1

Done: "Basil L. Contovounesios" <basil <at> contovou.net>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: "Basil L. Contovounesios" <basil <at> contovou.net>
To: Vladimir Kazanov <vekazanov <at> gmail.com>
Cc: 76372 <at> debbugs.gnu.org
Subject: bug#76372: 31.0.50; False () in ert-font-lock macro signatures
Date: Wed, 19 Feb 2025 21:15:50 +0100
Vladimir Kazanov [2025-02-18 19:41 +0000] wrote:

> Thanks for taking a look at ert-font-lock! I am on vacation right now
> so only have time for a brief review of the patch.

No worries.
I hope you have a nice vacation, and thanks for taking a look anyway.

> -     (insert ,str)
>       (,mode)
> +     (insert ,str)
>
> Not saying this is wrong but looks like something that might change
> things subtly. Do tests pass?

Yes, tests still pass (modulo those that were already failing):

$ make check
$ make TEST_LOAD_EL=no \
       test/{autoconf,{go,lua,rust}-ts-mode,ert{,-x,-font-lock}}-tests

If this were a general-purpose macro required to be robust, I would have
used e.g. macroexp-let2 to ensure the correct argument evaluation order.

Since this is a convenience used only in tests, I thought I could get
away with being lazy, but you caught me ;).

>> -  (skip-unless (featurep 'php-mode))
>> +  (skip-unless (fboundp 'php-mode))
>
> What difference does it make? featurep vs fboundp?

featurep returns non-nil if the file php-mode.el has already been
loaded, whereas fboundp returns non-nil if the function php-mode is
defined (even as an autoload).

During 'make check' it doesn't make a difference, since php-mode is not
even installed, let alone defined as a function or feature.

But if someone has php-mode.el installed via package-install, then the
function php-mode is automatically autoloaded.  If the test tries to
call the function php-mode, Emacs will know which file to load in order
to find the function's real definition.  So it is not necessary to have
php-mode.el already loaded (which involves an extra step by the user);
it suffices to have the php-mode package installed properly in order to
run the test.

Regardless of these details, fboundp is a more accurate test-skip
condition because all the test is doing is calling a function called
php-mode; it does not care about where that function is defined, and
specifically whether it's defined by a feature called php-mode.

>> -    "Test reading correct assertions from a file"
>> +  "Test reading correct assertions from a file"
>
> Similar to other docstrings, this asks for a period in the end.

Well spotted, thanks.

I've updated the patch locally, and am ready to apply further feedback,
or push it if there are no further comments.

-- 
Basil




This bug report was last modified 129 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.