Subject: Re: bug#61413: [PATCH] Make warnings show a "warning" emoji instead of a stop-sign

From: Konstantin Kharlamov <[email-masked]>
Date: Mon, 10 Mar 2025 20:59:43 +0300

On Mon, 2025-03-10 at 18:46 +0100, Kévin Le Gouguec wrote:
> Konstantin Kharlamov <[email-masked]> writes:
> 
> > On Mon, 2025-03-10 at 18:53 +0200, Eli Zaretskii wrote:
> > > > From: Konstantin Kharlamov <[email-masked]>
> > > > Cc: [email-masked], [email-masked], [email-masked], 
> > > > 	[email-masked], [email-masked]
> > > > Date: Mon, 10 Mar 2025 12:02:01 +0300
> > > > 
> > > > On Sat, 2025-03-08 at 18:05 +0100, Kévin Le Gouguec wrote:
> > > > > Eli Zaretskii <[email-masked]> writes:
> > > > > 
> > > > > > > > If you want me to install some patches, please post
> > > > > > > > them.
> > > > > > > 
> > > > > > > Thanks for the nudge; re-attaching the patch that makes
> > > > > > > button.el
> > > > > > > preserve help-echo strings; reviewed by Mauro in two
> > > > > > > parts¹², then
> > > > > > > squashed³.  I can install it if we think that fix is
> > > > > > > unambiguously good.
> > > > > > 
> > > > > > LGTM, thanks.
> > > > > 
> > > > > Just pushed, thanks for the review.
> > > > > 
> > > > > I'll defer to other participants regarding what should be
> > > > > done next -
> > > > > e.g. (a) should ⛔ be given a second chance now that it has a
> > > > > help-echo
> > > > > string, making its function more discoverable; (b) should we
> > > > > use another
> > > > > symbol; (c) should we eschew images entirely for that
> > > > > function & use a
> > > > > text button.
> > > > 
> > > > Sorry for stupid question, but… I just wanted to see how do
> > > > warnings
> > > > look now, and I figured I don't know how to produce the buffer
> > > > being
> > > > discussed 😅
> > > 
> > > Like this:
> > > 
> > >   M-: (display-warning 'warning "foo") RTE
> > 
> > Thanks! So, I'm looking at the buffer, and I don't see much
> > difference
> > compared to how it was before,
> 
> Right, the only changes that were committed at this stage impact the
> help-echo string, so the only visible difference happens if you hover
> over the no-entry sign, or invoke 'C-h .' with point on the sign.
> 
> >                                so my vote in preference of either
> > applying the patch (that one that started the discussion) or making
> > the
> > icon a button with text instead still stands.
> 
> ACK; FWIW I sent a patch combining both suggestions (using a warning
> sign - with either an SVG image or an emoji; using a text button for
> suppression) on this message:
> 
> <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=61413#108>
> 
> Screenshot:
> 
> <
> https://debbugs.gnu.org/cgi/bugreport.cgi?att=2;bug=61413;filename=Scr
> eenshot_20250223_163540.png;msg=108>

Looks nice! FTR, I'm looking at screenshot, and I think maybe it's
worth clarifying what I mean while saying making the icon a button. I
meant the visual appearance of a button, like the ones you can see in
customization menu for variables. Let me put it another way: if you
take your screenshot to a user who knows nothing about this buffer and
ask them "find all buttons on the screenshot" — do you think they'd
point at the warning sign and say "…and these are obviously buttons
too"? I bet they would not.

Other than that, your solution looks good; and disregarding the button
discussion it already looks better than what's currently on master.

> (Two things I dislike about that patch:
> 
> 1. the redundancy between the warning sign & the "Warning" text that
> comes after - feels like there is room for improvement, but would
> require delving into some warning-prefix-function plumbing I avoided
> up
> to this point;

"Better is the enemy of good" 😊 You're making a good point, but this
problem exists disregarding if your patch applied, and so fine to be
fixed separately. I would have made such change a separate patch
anyway, because to me it would seem an unrelated functional change.