bug#35058: [PATCH] Use display-graphic-p in more cases

[Alex <agrambot@gmail.com>]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: 35058@debbugs.gnu.org
>> Date: Sun, 31 Mar 2019 22:15:35 -0600
>> 
>> >>  	(if (and cua-rectangle-modifier-key
>> >> -		 (memq window-system '(x)))
>> >> +		 (eq window-system 'x))
>> >>  	    cua-rectangle-modifier-key
>> >>  	  'meta))
>> >>    ;; C-return always toggles rectangle mark
>> >
>> > Not sure about this one: what does a modifier key have to do with GUI
>> > display?
>> 
>> I wasn't sure either (I merely noticed the useless memq), but I just
>> checked the documentation of cua-rectangle-modifier-key, which states:
>> 
>>   On non-window systems, always use the meta modifier.
>> 
>> So I changed the eq call to display-graphics-p. Is it okay to follow the
>> docstring here?
>
> No, not IMO.  Doc strings can change because their purpose is
> documentation that's useful to users and programmers, whereas the
> issue in hand is ease of future maintenance.

Hmm, it seems that my terminal Emacs accepts the super modifier but not
the hyper modifier; is this a bug in Emacs? Should I remove the
window-system condition even if I can't get terminal Emacs to recognize
the hyper key?

> However, as you found out, some of the places still use window-system.
> At the time, they were left alone, mostly because it sounded gross to
> invent additional display-* functions which would have only one or two
> callers.  We could do this now, if we consider it important to get rid
> of more uses of window-system or framep; I still think it would be
> gross, but won't object if others think otherwise.  But let's not
> blindly use display-graphics-p to mean "anything that currently only
> happens on a GUI frame", because that's not what it stands for.  It
> stands for features that can _only_ be ever true on GUI displays, and
> can _never_ be implemented on any text-mode frame, and only for
> features for which there's no other display-* function that is more
> focused, like display-multi-font-p.

Thanks for describing the history here.

I don't think it's gross if there's only a couple callers; there could
be more callers in the future and in 3rd-party code, and even if there
weren't, a descriptive name should help better understanding of the
code.

>> >> @@ -2546,7 +2537,7 @@ blink-cursor-mode
>> >>    :init-value (not (or noninteractive
>> >>  		       no-blinking-cursor
>> >>  		       (eq system-type 'ms-dos)
>> >> -		       (not (memq window-system '(x w32 ns)))))
>> >> +		       (not (display-graphic-p))))
>> >
>> > Why would we want to connect blinking cursor with GUI displays?  These
>> > two are unrelated.
>> 
>> The definition of blink-cursor mode states:
>> 
>>   This command is effective only on graphical frames.  On text-only
>>   terminals, cursor blinking is controlled by the terminal."
>
> That's the _current_ situation.  But what would preclude the xterm
> developers, say, from adding a way of controlling that?  Nothing in
> particular, I'd say.

I agree that it's possible for the behaviour to be different eventually,
but in the meantime display-graphic-p describes the current situation
and intent better than the explicit memq does.

If text-only terminals gain the ability to control this behaviour, then
that's the time to remove the check, no?

>> >>              ;; This is a serious problem for trying to handle multiple
>> >>              ;; frame types at once.  We want this text to be invisible
>> >>              ;; on frames that can display the font above.
>> >> -            (when (memq (framep (selected-frame)) '(x pc w32 ns))
>> >> +            (unless (memq (framep (selected-frame)) '(nil t))
>> >>                (add-text-properties (1- (match-beginning 2)) (match-end 2)
>> >>                                     '(invisible t front-sticky nil rear-nonsticky t))))))
>> >
>> > Not sure here, but if this about fonts, then display-multi-font-p is a
>> > better test.
>> 
>> Is multi-font equivalent to supporting invisible text?
>
> The comment above talks about fonts; I didn't read the code well
> enough to understand what it's about.  Maybe display-multi-font-p is a
> better predicate here, not sure.

I believe it is, since it appears to be referring to the ***** that is
visible below the Info title only in text-terminals. I've altered the
patch to use display-multi-font-p.

>> > normal-erase-is-backspace-mode is entirely unrelated to display being
>> > GUI, so I don't think this is right.
>> 
>> The docstring of normal-erase-is-backspace-mode mentions window-system
>> several times, so I don't see the issue.
>
> I hope now you understand my motivation better.

I believe so. I'd like to replace the memq with a descriptive name, but
I'm not sure what to call it; display-ascii-encoding-p to denote that
the display can not differentiate between, e.g., C-m and RET?

>> >> diff --git a/lisp/window.el b/lisp/window.el
>> >> index b769be0633..b622debd51 100644
>> >> --- a/lisp/window.el
>> >> +++ b/lisp/window.el
>> >> @@ -9351,7 +9351,7 @@ handle-select-window
>> >>        ;; we might get two windows with an active cursor.
>> >>        (select-window window)
>> >>        (cond
>> >> -       ((or (not (memq (window-system frame) '(x w32 ns)))
>> >> +       ((or (memq (window-system frame) '(nil t pc))
>> >>              (not focus-follows-mouse)
>> >>              ;; Focus FRAME if it's either a child frame or an ancestor
>> >>              ;; of the frame switched from.
>> >
>> > This is again a reversion of logic which I think is a change for the
>> > worse.
>> 
>> Would it be okay to forward-declare display-graphic-p and use it here as
>> well?
>
> Not IMO, because this is again about selection with a mouse, something
> that can (and is) supported on some TTY frames.  We could use the
> hypothetical display-iconifiable-p (but we should then find a better
> name, something about selecting/raising frames perhaps).

display-multi-focus-p? But maybe that implies that it can focus on
multiple frames simultaneously.

What about using display-multi-frame-p? Could there be some system
that allows multiple frames, but no way to select between them?