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

Previous Next

Full log

View this message in rfc822 format

From: Vladimir Kazanov <vekazanov <at> gmail.com>
To: "Basil L. Contovounesios" <basil <at> contovou.net>
Cc: 76372 <at> debbugs.gnu.org
Subject: bug#76372: 31.0.50; False () in ert-font-lock macro signatures
Date: Thu, 20 Feb 2025 08:23:41 +0000

Feel free to push,

Thanks for looking into this!

On Wed, 19 Feb 2025 at 20:15, Basil L. Contovounesios
<basil <at> contovou.net> wrote:
>
> 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



-- 
Regards,

Vladimir Kazanov

Previous Next

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