[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#35058: [PATCH] Use display-graphic-p in more cases
From: |
Alex |
Subject: |
bug#35058: [PATCH] Use display-graphic-p in more cases |
Date: |
Tue, 02 Apr 2019 11:05:09 -0600 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) |
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?
- bug#35058: [PATCH] Use display-graphic-p in more cases, Alex, 2019/04/01
- bug#35058: [PATCH] Use display-graphic-p in more cases, Eli Zaretskii, 2019/04/01
- bug#35058: [PATCH] Use display-graphic-p in more cases, Drew Adams, 2019/04/01
- bug#35058: [PATCH] Use display-graphic-p in more cases,
Alex <=
- bug#35058: [PATCH] Use display-graphic-p in more cases, Eli Zaretskii, 2019/04/02
- bug#35058: [PATCH] Use display-graphic-p in more cases, Alex, 2019/04/02
- bug#35058: [PATCH] Use display-graphic-p in more cases, Eli Zaretskii, 2019/04/02
- bug#35058: [PATCH] Use display-graphic-p in more cases, Alex, 2019/04/03
- bug#35058: [PATCH] Use display-graphic-p in more cases, Eli Zaretskii, 2019/04/03
- bug#35058: [PATCH] Use display-graphic-p in more cases, Alex, 2019/04/03
- bug#35058: [PATCH] Use display-graphic-p in more cases, Eli Zaretskii, 2019/04/05
- bug#35058: [PATCH] Use display-graphic-p in more cases, Alex, 2019/04/05
- bug#35058: [PATCH] Use display-graphic-p in more cases, Eli Zaretskii, 2019/04/05
- bug#35058: [PATCH] Use display-graphic-p in more cases, Alex, 2019/04/07