bug-gnu-emacs
[Top][All Lists]
Advanced

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

bug#35476: font-lock-{append, prepend}-text-property and anonymous faces


From: Noam Postavsky
Subject: bug#35476: font-lock-{append, prepend}-text-property and anonymous faces
Date: Sun, 12 May 2019 15:09:36 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux)

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.





reply via email to

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