bug#64075: 28.2; ispell broken on uncolored terminals

[Eli Zaretskii]

> From: Al Petrofsky <al@petrofsky.org>
> Date: Thu, 15 Jun 2023 19:19:23 -0400
> Cc: gregory@heytings.org, 64075@debbugs.gnu.org
> 
>    emacs-29.0.91 -Q -nw --color=no
>    C-x b x RET
>    M-x enriched-mode RET
>    M-o b stonks M-$ SPC
> 
> At this point, "stonks" should have been unchanged by the M-$ SPC, and
> therefore should still be bold, but instead only the "s" is bold and
> the "tonks" has the default face.  Less importantly, while the ispell
> choices were displayed, "stonks" should have been in inverse-video,
> but it was not.

Your expectations are wrong: no one said pasting arbitrary text in
Enriched mode over a word should necessarily retain the face
information from the original miss-spelled word.

> As was the case for the original recipe, changing "(display-color-p)"
> to "t" in ispell-highlight-spelling-error fixes the bugs.

Because that uses overlays, so it doesn't interfere with faces.  But
you haven't tested it with any other feature that uses overlays with
face information.  So your testing is not balanced: it is designed to
emphasize the disadvantages of only one method, and disregards the
disadvantages of the other.

> > It's a working code whose replacement (basically, a cleanup) will mean
> > extra work for us, and all that for quite rare situations.  Based on
> > my long experience with Emacs, it also means some subtle bugs in some
> > even rarer use cases, which will take years to find and fix.
> 
> As you said in bug #56219, it's code that hasn't worked in any release
> in the last 15 years (now 16 years).

That's not what I said in that bug.

> What I'm saying is it's also
> code for a special case that hasn't existed for 22 years (that the
> terminal has an "so" capability but the display code can't render
> inverse-video faces on it), and that's why it makes more sense to me
> to simply eliminate this code and let monochrome terminals be handled
> by ispell-highlight-spelling-error-overlay (which is working code),
> rather than go in and try to fix the logic in a function that is a
> self-labeled "kludge" that depends on details of the low-level
> redisplay engine, and hope no new subtle bug has been introduced.

I've considered your proposal and decided against removing this code.
Please don't keep pushing for it, as I won't change my mind just
because you are reiterating the same arguments.

> I can easily use the defaults, or easily change the face to suit my
> preferences on whatever type of terminal I use most, but to get
> support for different types of terminals in a custom manner takes more
> work.

That was exactly my point.  So if we want to make ispell.el work
better in these cases, we need more complex setting of the faces used
in this case, and in particular to depend less on (possibly
customized) other faces.

> Is this situation so bad that it would be preferable for isearch to be
> hard-coded to always use inverse video on monochrome displays?  Is
> that your position?  I think I prefer it as is, and would prefer that
> ispell worked the same way.

My position is as I already explained it.  let me reiterate it:

  . I'm okay with adding more conditions to use the overlay code, by
    testing terminal capabilities other that display-color-p
  . Each such capability should have a distinct face to go with it,
    and that face should use the corresponding display capabilities
    without inheriting from other, more general faces

It is possible that, if we support enough display capabilities in the
above way, the set of terminals that use
ispell-highlight-spelling-error-generic will become much smaller, or
even empty.  But the difference is that we will have a comprehensive
and sound solution based on specific terminal capabilities instead of
a hack that works only in some situations.

> I do wonder if it would make sense to make :inverse-video t in all the
> defaults for the isearch face, and swap all the default :background
> and :foreground properties.  That would result in no change to the
> default appearances, with the benefit that after a simple
> customization of the colors during an emacs session on a color
> terminal, emacs sessions on monochrome terminals would still use
> inverse-video for isearch (and ispell).

Sorry, we will not change the defaults for a face as general and
widely-used as 'isearch', because such a change will cause annoyance
from many users.  Definitely not due to marginal use cases such as
this one.  (You can, of course, make such face customizations
locally.)  This problem should be solved where it happens: in
ispell.el, not by changing a much wider used face.