Package: emacs;
Reported by: Alex <agrambot <at> gmail.com>
Date: Sat, 30 Mar 2019 23:39:01 UTC
Severity: wishlist
Tags: patch
Done: Alex <agrambot <at> gmail.com>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Alex <agrambot <at> gmail.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 35058 <at> debbugs.gnu.org Subject: bug#35058: [PATCH] Use display-graphic-p in more cases Date: Tue, 02 Apr 2019 11:05:09 -0600
Eli Zaretskii <eliz <at> gnu.org> writes: >> From: Alex <agrambot <at> gmail.com> >> Cc: 35058 <at> 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?
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.