GNU bug report logs - #35476
[PATCH] 27.0.50; font-lock-{append,prepend}-text-property and anonymous faces

Previous Next

Full log

Message #20 received at 35476 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 35476 <at> debbugs.gnu.org
Subject: Re: bug#35476: font-lock-{append,
 prepend}-text-property and anonymous faces
Date: Sun, 12 May 2019 15:09:36 -0400

Looks basically good, I just have a few nitpicks below.

> Subject: [PATCH 1/2] Stop splicing anonymous faces in
>  font-lock-append-text-property
>
> This is the same fix as f478082, which was only applied to

It's best to avoid using hashes in commit messages, as they're
translated to ChangeLog files which might read from the tarball (i.e.,
without a git repo to hand).  CONTRIBUTE talks about using "action
stamps" but I think date+title is more readable.  Which would be:
2019-04-29 "Refrain from splicing anonymous faces in text properties".

> +(provide 'font-lock-tests)

I don't think there is any reason to `provide' a feature in a test file
(I know some other test files do that, but I don't see why), test files
are generally not loaded via require.

> Subject: [PATCH 2/2] Extract common code for adding text properties

> +      (let ((new-value (if append
> +                           (append (if (listp prev) prev (list prev)) val)
> +                         (append val (if (listp prev) prev (list prev))))))

I would suggest to factor out the (if (listp prev) prev (list prev))
from these expressions.

Previous Next

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