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).
>