On Wed, Mar 13, 2024 at 6:18 AM Eli Zaretskii <eliz@gnu.org> wrote:

> From: Elijah G <eg642616@gmail.com>
> Date: Tue, 12 Mar 2024 11:22:04 -0600
> Cc: emacs-devel@gnu.org
>
> Thank you, I've fixed my patch.

Thanks, I have a few more comments below.

> Also I merged every flymake *-bitmap variable
> into a single user option.

I think this is a mistake: it makes this change backward-incompatible,
in the sense that users who had customizations of the options you are
removing will now have to rework their customizations. We try to
avoid backward-incompatible changes like this as much as possible. It
doesn't sound to me like this part of the patch is strictly needed, is
it?

You are right, i was planning something like marking those variables as deprecated for 30,

but since it makes backward incompatible think it can be too early to do it in this patch,

However I find merging them easier to customize instead of defining them separately.

If these variables are necessary, I've now reverted the changes in the patch.

> + :type '(choice (const :tag "Use Fringes" fringes)
> + (const :tag "Use Margins "margins)
^^
A typo there.

Thanks, I've fixed it.

> +(defcustom flymake-margin-indicators-string '((error "‼" compilation-error)

This is a non-ASCII character, so we should either use its ASCII
equivalent "!!" or test the display for being able to show it on the
screen (since it is likely the margin indicator will be used on TTY
frames).

Please also accompany your patch with the ChangeLog-style commit log
message (see the file CONTRIBUTE for the details).