From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#35058: [PATCH] Use display-graphic-p in more cases Date: Sun, 31 Mar 2019 18:37:57 +0300 Message-ID: <83tvfjgilm.fsf@gnu.org> References: <8736n4ndav.fsf@gmail.com> Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="56604"; mail-complaints-to="usenet@blaine.gmane.org" Cc: 35058@debbugs.gnu.org To: Alex Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sun Mar 31 17:38:17 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hAcXI-000EaY-Os for geb-bug-gnu-emacs@m.gmane.org; Sun, 31 Mar 2019 17:38:17 +0200 Original-Received: from localhost ([127.0.0.1]:50052 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAcXH-0002Ch-Iw for geb-bug-gnu-emacs@m.gmane.org; Sun, 31 Mar 2019 11:38:15 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:57994) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAcX6-0002BL-Gw for bug-gnu-emacs@gnu.org; Sun, 31 Mar 2019 11:38:09 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hAcX4-0006I4-7R for bug-gnu-emacs@gnu.org; Sun, 31 Mar 2019 11:38:04 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:53107) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hAcX4-0006HC-03 for bug-gnu-emacs@gnu.org; Sun, 31 Mar 2019 11:38:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hAcX3-00055v-PY for bug-gnu-emacs@gnu.org; Sun, 31 Mar 2019 11:38:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 31 Mar 2019 15:38:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 35058 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 35058-submit@debbugs.gnu.org id=B35058.155404668019576 (code B ref 35058); Sun, 31 Mar 2019 15:38:01 +0000 Original-Received: (at 35058) by debbugs.gnu.org; 31 Mar 2019 15:38:00 +0000 Original-Received: from localhost ([127.0.0.1]:38418 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hAcX1-00055f-RI for submit@debbugs.gnu.org; Sun, 31 Mar 2019 11:38:00 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:43581) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hAcX0-00055S-7c for 35058@debbugs.gnu.org; Sun, 31 Mar 2019 11:37:59 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:57422) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAcWv-0006Bj-04; Sun, 31 Mar 2019 11:37:53 -0400 Original-Received: from [176.228.60.248] (port=4568 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1hAcWt-0004vK-Op; Sun, 31 Mar 2019 11:37:52 -0400 In-reply-to: <8736n4ndav.fsf@gmail.com> (message from Alex on Sat, 30 Mar 2019 17:38:16 -0600) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:156976 Archived-At: > From: Alex > Date: Sat, 30 Mar 2019 17:38:16 -0600 > > >From b760955517f9c39b13353bdf5ae49f25fbadc66f Mon Sep 17 00:00:00 2001 > From: Alexander Gramiak > Date: Sat, 30 Mar 2019 16:53:11 -0600 > Subject: [PATCH] Use display-graphic-p in more cases Thanks for working on this. In general, most places where we don't use display-graphic-p do that for a reason, see the comments below. The general idea of display-graphic-p and other similar predicates and low-level APIs is that they hide these details from the callers, and their names explain themselves. This is why we have several predicates whose implementation is almost identical: each one of them is supposed to provide a test for a different kind of functionalities, even if currently the same frame types return non-nil from all of these predicates. The point is to allow easier changes in the future when additional frame types acquire capabilities they don't have today. We already had such episodes with the mouse and menus. But some of the places you found can indeed use display-graphic-p nowadays. > diff --git a/lisp/disp-table.el b/lisp/disp-table.el > index 476c0cb986..a3e8892c51 100644 > --- a/lisp/disp-table.el > +++ b/lisp/disp-table.el > @@ -176,7 +176,7 @@ standard-display-g1 > "Display character C as character SC in the g1 character set. > This function assumes that your terminal uses the SO/SI characters; > it is meaningless for an X frame." > - (if (memq window-system '(x w32 ns)) > + (if (display-graphic-p) > (error "Cannot use string glyphs in a windowing system")) > (or standard-display-table > (setq standard-display-table (make-display-table))) > @@ -188,7 +188,7 @@ standard-display-graphic > "Display character C as character GC in graphics character set. > This function assumes VT100-compatible escapes; it is meaningless for an > X frame." > - (if (memq window-system '(x w32 ns)) > + (if (display-graphic-p) > (error "Cannot use string glyphs in a windowing system")) > (or standard-display-table > (setq standard-display-table (make-display-table))) > @@ -276,7 +276,7 @@ standard-display-european > (progn > (standard-display-default > (unibyte-char-to-multibyte 160) (unibyte-char-to-multibyte 255)) > - (unless (or (memq window-system '(x w32 ns))) > + (unless (display-graphic-p) > (and (terminal-coding-system) > (set-terminal-coding-system nil)))) > > @@ -289,7 +289,7 @@ standard-display-european > ;; unless some other has been specified. > (if (equal current-language-environment "English") > (set-language-environment "latin-1")) > - (unless (or noninteractive (memq window-system '(x w32 ns))) > + (unless (or noninteractive (display-graphic-p)) > ;; Send those codes literally to a character-based terminal. > ;; If we are using single-byte characters, > ;; it doesn't matter which coding system we use. This part is fine by me. > diff --git a/lisp/emulation/cua-base.el b/lisp/emulation/cua-base.el > index 302ef12386..461c4ea9aa 100644 > --- a/lisp/emulation/cua-base.el > +++ b/lisp/emulation/cua-base.el > @@ -1238,7 +1238,7 @@ cua--init-keymaps > ;; Cache actual rectangle modifier key. > (setq cua--rectangle-modifier-key > (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? > diff --git a/lisp/faces.el b/lisp/faces.el > index ab6c384c80..fa526c3506 100644 > --- a/lisp/faces.el > +++ b/lisp/faces.el > @@ -55,6 +55,7 @@ term-file-aliases > :group 'terminals > :version "25.1") > > +(declare-function display-graphic-p "frame" (&optional display)) > (declare-function xw-defined-colors "term/common-win" (&optional frame)) > > (defvar help-xref-stack-item) > @@ -1239,7 +1240,7 @@ read-face-attribute > ;; explicitly in VALID, using color approximation code > ;; in tty-colors.el. > (when (and (memq attribute '(:foreground :background)) > - (not (memq (window-system frame) '(x w32 ns))) > + (not (display-graphic-p frame)) > (not (member new-value > '("unspecified" > "unspecified-fg" "unspecified-bg")))) > @@ -1833,7 +1834,7 @@ defined-colors > The value may be different for frames on different display types. > If FRAME doesn't support colors, the value is nil. > If FRAME is nil, that stands for the selected frame." > - (if (memq (framep (or frame (selected-frame))) '(x w32 ns)) > + (if (display-graphic-p frame) > (xw-defined-colors frame) > (mapcar 'car (tty-color-alist frame)))) > (defalias 'x-defined-colors 'defined-colors) > @@ -1877,7 +1878,7 @@ color-defined-p > > If FRAME is omitted or nil, use the selected frame." > (unless (member color '(unspecified "unspecified-bg" "unspecified-fg")) > - (if (member (framep (or frame (selected-frame))) '(x w32 ns)) > + (if (display-graphic-p frame) > (xw-color-defined-p color frame) > (numberp (tty-color-translate color frame))))) > (defalias 'x-color-defined-p 'color-defined-p) > @@ -1903,7 +1904,7 @@ color-values > (cond > ((member color '(unspecified "unspecified-fg" "unspecified-bg")) > nil) > - ((memq (framep (or frame (selected-frame))) '(x w32 ns)) > + ((display-graphic-p frame) > (xw-color-values color frame)) > (t > (tty-color-values color frame)))) > @@ -1917,7 +1918,7 @@ display-color-p > The optional argument DISPLAY specifies which display to ask about. > DISPLAY should be either a frame or a display name (a string). > If omitted or nil, that stands for the selected frame's display." > - (if (memq (framep-on-display display) '(x w32 ns)) > + (if (display-graphic-p display) > (xw-display-color-p display) > (tty-display-color-p display))) > (defalias 'x-display-color-p 'display-color-p) > @@ -1928,12 +1929,9 @@ display-grayscale-p > "Return non-nil if frames on DISPLAY can display shades of gray. > DISPLAY should be either a frame or a display name (a string). > If omitted or nil, that stands for the selected frame's display." > - (let ((frame-type (framep-on-display display))) > - (cond > - ((memq frame-type '(x w32 ns)) > - (x-display-grayscale-p display)) > - (t > - (> (tty-color-gray-shades display) 2))))) > + (if (display-graphic-p display) > + (x-display-grayscale-p display) > + (> (tty-color-gray-shades display) 2))) > > (defun read-color (&optional prompt convert-to-RGB allow-empty-name msg) > "Read a color name or RGB triplet. These are okay, I think. > diff --git a/lisp/frame.el b/lisp/frame.el > index 6cb1247372..f5ad3152a0 100644 > --- a/lisp/frame.el > +++ b/lisp/frame.el > @@ -974,7 +974,7 @@ select-frame-set-input-focus > (select-frame frame norecord) > (raise-frame frame) > ;; Ensure, if possible, that FRAME gets input focus. > - (when (memq (window-system frame) '(x w32 ns)) > + (when (display-graphic-p frame) > (x-focus-frame frame)) I don't see what display being GUI have to do with frame focus redirection. > @@ -1027,11 +1027,11 @@ suspend-frame > "Do whatever is right to suspend the current frame. > Calls `suspend-emacs' if invoked from the controlling tty device, > `suspend-tty' from a secondary tty device, and > -`iconify-or-deiconify-frame' from an X frame." > +`iconify-or-deiconify-frame' from a graphical frame." > (interactive) > (let ((type (framep (selected-frame)))) > (cond > - ((memq type '(x ns w32)) (iconify-or-deiconify-frame)) > + ((display-graphic-p display) (iconify-or-deiconify-frame)) > ((eq type t) > (if (controlling-tty-p) > (suspend-emacs) Likewise here: there's no reason apriori for any logical relation to exist between GUI display and iconifying/deiconifying a frame. > @@ -1895,7 +1895,7 @@ display-graphic-p > that use a window system such as X, and false for text-only terminals. > DISPLAY can be a display name, a frame, or nil (meaning the selected > frame's display)." > - (not (null (memq (framep-on-display display) '(x w32 ns))))) > + (not (memq (framep-on-display display) '(nil t pc)))) I prefer the current variant, as it shows the frame types explicitly. Doing that for frames that are NOT something means people will have problems looking up these dependencies when they need to. > (defun display-images-p (&optional display) > "Return non-nil if DISPLAY can display images. > @@ -1933,12 +1933,9 @@ display-screens > "Return the number of screens associated with DISPLAY. > DISPLAY should be either a frame or a display name (a string). > If DISPLAY is omitted or nil, it defaults to the selected frame's display." > - (let ((frame-type (framep-on-display display))) > - (cond > - ((memq frame-type '(x w32 ns)) > - (x-display-screens display)) > - (t > - 1)))) > + (if (display-graphic-p display) > + (x-display-screens display) > + 1)) > > (declare-function x-display-pixel-height "xfns.c" (&optional terminal)) > > @@ -1953,12 +1950,9 @@ display-pixel-height > refers to the pixel height for all physical monitors associated > with DISPLAY. To get information for each physical monitor, use > `display-monitor-attributes-list'." > - (let ((frame-type (framep-on-display display))) > - (cond > - ((memq frame-type '(x w32 ns)) > - (x-display-pixel-height display)) > - (t > - (frame-height (if (framep display) display (selected-frame))))))) > + (if (display-graphic-p display) > + (x-display-pixel-height display) > + (frame-height (if (framep display) display (selected-frame))))) > > (declare-function x-display-pixel-width "xfns.c" (&optional terminal)) > > @@ -1973,12 +1967,9 @@ display-pixel-width > refers to the pixel width for all physical monitors associated > with DISPLAY. To get information for each physical monitor, use > `display-monitor-attributes-list'." > - (let ((frame-type (framep-on-display display))) > - (cond > - ((memq frame-type '(x w32 ns)) > - (x-display-pixel-width display)) > - (t > - (frame-width (if (framep display) display (selected-frame))))))) > + (if (display-graphic-p display) > + (x-display-pixel-width display) > + (frame-width (if (framep display) display (selected-frame))))) > > (defcustom display-mm-dimensions-alist nil > "Alist for specifying screen dimensions in millimeters. > @@ -2013,7 +2004,7 @@ display-mm-height > refers to the height in millimeters for all physical monitors > associated with DISPLAY. To get information for each physical > monitor, use `display-monitor-attributes-list'." > - (and (memq (framep-on-display display) '(x w32 ns)) > + (and (display-graphic-p display) > (or (cddr (assoc (or display (frame-parameter nil 'display)) > display-mm-dimensions-alist)) > (cddr (assoc t display-mm-dimensions-alist)) > @@ -2034,7 +2025,7 @@ display-mm-width > refers to the width in millimeters for all physical monitors > associated with DISPLAY. To get information for each physical > monitor, use `display-monitor-attributes-list'." > - (and (memq (framep-on-display display) '(x w32 ns)) > + (and (display-graphic-p display) > (or (cadr (assoc (or display (frame-parameter nil 'display)) > display-mm-dimensions-alist)) > (cadr (assoc t display-mm-dimensions-alist)) > @@ -2078,12 +2069,12 @@ display-planes > If DISPLAY is omitted or nil, it defaults to the selected frame's display." > (let ((frame-type (framep-on-display display))) > (cond > - ((memq frame-type '(x w32 ns)) > - (x-display-planes display)) > ((eq frame-type 'pc) > 4) > + ((memq frame-type '(nil t)) > + (truncate (log (length (tty-color-alist)) 2))) > (t > - (truncate (log (length (tty-color-alist)) 2)))))) > + (x-display-planes display))))) > > (declare-function x-display-color-cells "xfns.c" (&optional terminal)) > > @@ -2093,12 +2084,12 @@ display-color-cells > If DISPLAY is omitted or nil, it defaults to the selected frame's display." > (let ((frame-type (framep-on-display display))) > (cond > - ((memq frame-type '(x w32 ns)) > - (x-display-color-cells display)) > ((eq frame-type 'pc) > 16) > + ((memq frame-type '(nil t)) > + (tty-display-color-cells display)) > (t > - (tty-display-color-cells display))))) > + (x-display-color-cells display))))) > > (declare-function x-display-visual-class "xfns.c" (&optional terminal)) > > @@ -2110,13 +2101,13 @@ display-visual-class > If DISPLAY is omitted or nil, it defaults to the selected frame's display." > (let ((frame-type (framep-on-display display))) > (cond > - ((memq frame-type '(x w32 ns)) > - (x-display-visual-class display)) > + ((not frame-type) > + 'static-gray) > ((and (memq frame-type '(pc t)) > (tty-display-color-p display)) > 'static-color) > (t > - 'static-gray)))) > + (x-display-visual-class display))))) > > (declare-function x-display-monitor-attributes-list "xfns.c" > (&optional terminal)) I prefer not to change these. These APIs are the lowest level of testing for the respective features, so I prefer them to show explicitly on what types of frames we support them and how. Adding indirection through display-graphic-p doesn't help here, because these interfaces are siblings of display-graphic-p. > @@ -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. > diff --git a/lisp/info.el b/lisp/info.el > index f2a064abb6..4954916969 100644 > --- a/lisp/info.el > +++ b/lisp/info.el > @@ -4768,7 +4768,7 @@ Info-fontify-node > ;; 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. > diff --git a/lisp/simple.el b/lisp/simple.el > index f76f31ad14..6651ee2fc5 100644 > --- a/lisp/simple.el > +++ b/lisp/simple.el > @@ -8682,7 +8682,7 @@ normal-erase-is-backspace-setup-frame > (and (not noninteractive) > (or (memq system-type '(ms-dos windows-nt)) > (memq window-system '(w32 ns)) > - (and (memq window-system '(x)) > + (and (eq window-system 'x) > (fboundp 'x-backspace-delete-keys-p) > (x-backspace-delete-keys-p)) > ;; If the terminal Emacs is running on has erase char > @@ -8728,7 +8728,7 @@ normal-erase-is-backspace-mode > (let ((enabled (eq 1 (terminal-parameter > nil 'normal-erase-is-backspace)))) > > - (cond ((or (memq window-system '(x w32 ns pc)) > + (cond ((or window-system > (memq system-type '(ms-dos windows-nt))) > (let ((bindings > '(([M-delete] [M-backspace]) normal-erase-is-backspace-mode is entirely unrelated to display being GUI, so I don't think this is right. > 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. Thanks.