From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Alex Newsgroups: gmane.emacs.bugs Subject: bug#35058: [PATCH] Use display-graphic-p in more cases Date: Sun, 31 Mar 2019 22:15:35 -0600 Message-ID: <875zry9x94.fsf@gmail.com> References: <8736n4ndav.fsf@gmail.com> <83tvfjgilm.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="157432"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) Cc: 35058@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Mon Apr 01 06:16:28 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 1hAoMz-000ecn-OE for geb-bug-gnu-emacs@m.gmane.org; Mon, 01 Apr 2019 06:16:26 +0200 Original-Received: from localhost ([127.0.0.1]:52381 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAoMt-0000lg-Bt for geb-bug-gnu-emacs@m.gmane.org; Mon, 01 Apr 2019 00:16:19 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:46813) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAoMj-0000lO-D9 for bug-gnu-emacs@gnu.org; Mon, 01 Apr 2019 00:16:11 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hAoMe-0004s3-3x for bug-gnu-emacs@gnu.org; Mon, 01 Apr 2019 00:16:06 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:53433) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hAoMc-0004rc-8y for bug-gnu-emacs@gnu.org; Mon, 01 Apr 2019 00:16:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hAoMc-0008U6-11 for bug-gnu-emacs@gnu.org; Mon, 01 Apr 2019 00:16:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Alex Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 01 Apr 2019 04:16: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.155409214932593 (code B ref 35058); Mon, 01 Apr 2019 04:16:01 +0000 Original-Received: (at 35058) by debbugs.gnu.org; 1 Apr 2019 04:15:49 +0000 Original-Received: from localhost ([127.0.0.1]:38744 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hAoMO-0008Tc-9M for submit@debbugs.gnu.org; Mon, 01 Apr 2019 00:15:48 -0400 Original-Received: from mail-pl1-f174.google.com ([209.85.214.174]:44914) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hAoML-0008TP-PK for 35058@debbugs.gnu.org; Mon, 01 Apr 2019 00:15:46 -0400 Original-Received: by mail-pl1-f174.google.com with SMTP id g12so3798517pll.11 for <35058@debbugs.gnu.org>; Sun, 31 Mar 2019 21:15:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=/iWBnlAWHi+7/LDERHMf/GH1WODSpha9To2zpvW9L4E=; b=BYDp5Ws2uIS0jgb+Bmkil2DtfOruMEQo+HBcX1joBc/LQ81JaLkWxrIVJxDdf9eIEA BSpbcIoRvGWzun6/S474jbA2ox7qO0pFBZi1G/XTEYLWgszX0w5oipBT1jule+hlakop lQDQSsPPzvLKBbZXHRuXVZ9tjUuL/Q/MQWch3eJewOHlOxiClQtStiQxEHGxeIauPGEa p/yCSMUR3LT3JqwflzuIECDIsC9CFybwcy2res50H78+UYWjW218m5iCF61LYRMZ7Mra ROwoDHUxjJEgFFzNOJD570pWkEp/9wik/3mS8a7diuQsmGWV6m6j4HVF3D44KFSlTgyP WUoA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=/iWBnlAWHi+7/LDERHMf/GH1WODSpha9To2zpvW9L4E=; b=jg6rScxJhXlvXW9uYkSCGVcNahe4wdWiC1y6czC/HRtzFjaan75Qu9qh+qvIOUSSIV rhnslE0e8NAziWI1kmI3aF/w2cZ/EydEa3fSjMRsZ+XLmrSUwv21uZgXAqTnyPI1Wb0i eHAxeBMcaQ5H2brjcqtu9T9e2vVF623exEDpGhleWX0uyJ7f511DtKVSBoz2iI5jFsF6 OduP/JQXcndHuMDNKluDxXmGnnL0m/hwp9pJzKaIeMbnlR1GJjv4XPAZ+9ERhCEwTtNQ ZjvJQcp9NPoZp/KDsTpIOsjgsH7hoMGsI9BtKMpWY9AFJaCDtgDd3r9z7WysCiyOaVwX AhqQ== X-Gm-Message-State: APjAAAWrELWLU5NCx1XGMYoWQ5Sd9rOXUd9Tt82vvTGCwvvZSkEf62X0 K4jbsoJ52iTuZwobDvm+zgyRLroW X-Google-Smtp-Source: APXvYqw25EXqAlGF9wT29WI5Lb0NSSH6ly+4Nqn5o/Q5QDJFlRiUVKCDHYVg5rznfIGBLfBKZeQb/A== X-Received: by 2002:a17:902:7794:: with SMTP id o20mr15410070pll.189.1554092139365; Sun, 31 Mar 2019 21:15:39 -0700 (PDT) Original-Received: from lylat ([2604:3d09:e37f:1500:1a72:4878:e793:7302]) by smtp.gmail.com with ESMTPSA id i126sm12173603pfb.15.2019.03.31.21.15.37 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sun, 31 Mar 2019 21:15:38 -0700 (PDT) In-Reply-To: <83tvfjgilm.fsf@gnu.org> (Eli Zaretskii's message of "Sun, 31 Mar 2019 18:37:57 +0300") 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:157005 Archived-At: Eli Zaretskii writes: >> 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? 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? >> 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. The x-focus-frame here is the GUI-specific code related to frame focus that calls x_focus_frame, which is only defined when HAVE_WINDOW_SYSTEM is defined. This is one of the procedures that I'm planning to rename with a gui prefix, so I believe display-graphic-p makes sense here. >> @@ -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. What would (de)iconifying be in non-GUI displays? If there is indeed no logical relation, then what about a new display-iconifiable-p that is made an alias? >> @@ -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. I'm not sure how much of a problem that would be if the return values of framep-on-display were listed in its docstring, but I guess it's not really an issue leaving this one alone. >> (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. If all graphical displays support these features currently, and if it's reasonable to presume that any new graphical display types would also support them, then I don't see why it would be a problem to use display-graphics-p in these cases. If the unexpected occurs, then there would only be a few definitions to change at most. For example, is it really wrong to presume that all graphical displays can retrieve pixel height/width? >> @@ -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." So it seems reasonable to use display-graphic-p here. >> 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. Is multi-font equivalent to supporting invisible text? If so, then I'll use that and (or ... (eq window-system 'pc)). >> 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. The docstring of normal-erase-is-backspace-mode mentions window-system several times, so I don't see the issue. >> 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?