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

[Alex <agrambot@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2003 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: 35058@debbugs.gnu.org
>> Date: Tue, 02 Apr 2019 23:14:06 -0600
>> 
>> >> > If it is very inconvenient to use hyper on text terminals, then I
>> >> > think we shouldn't require users to do that.
>> >> 
>> >> It wouldn't be a matter of requiring, but instead forcing the modifier
>> >> back to 'meta if both in terminal Emacs and cua-rectangle-modifier-key
>> >> is 'hyper.
>> >
>> > Sorry, I don't understand.  Can you show how would you like to remove
>> > the condition?
>> 
>> I don't agree with my position in my last email since I found out that
>> my terminal has a special workaround for 'super.
>> 
>> Removing the condition would just mean always using
>> cua-rectangle-modifier-key in cua--init-keymaps even if in a terminal:
>> 
>> (setq cua--rectangle-modifier-key cua-rectangle-modifier-key)
>> 
>> This would mean that terminal users that can't use super/hyper modifiers
>> easily would might find it confusing, but the current behaviour is a bit
>> confusing as is.
>
> I think the use case behind that condition is of a user who uses both
> GUI and TTY frames, and have cua-rectangle-modifier-key customized to
> something that doesn't easily work on TTY.  Perhaps the right solution
> would be to have 2 separate defcustom's, one each for every frame type.

Okay, I did that below.

>> How about display-symbol-keys-p (if TTY frames can't support symbols in
>> key sequences)? 
>
> Fine with me, thanks.  Although having 2 defcustom's might eliminate
> the need for the test and for the predicate.

The 2 defcustom situation applies to the CUA code, not for
normal-erase-is-backspace-mode (where display-symbol-keys-p is used).

I've attached a patch series below (including the logb change Basil
recommended). Are these okay to apply?

The first patch does still use display-graphic-p in a few cases in
frame.el for the predicates concerning pixel dimensions. Do you really
want them to use memq instead?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-display-graphic-p-and-display-multi-frame-p-in-m.patch --]
[-- Type: text/x-patch, Size: 11338 bytes --]

From a45ebab6020152a515dd92d04995e74ab52a6978 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Wed, 3 Apr 2019 13:57:16 -0600
Subject: [PATCH 1/4] Use display-graphic-p and display-multi-frame-p in more
 cases

* lisp/disp-table.el:
* lisp/faces.el:
* lisp/frame.el:
* lisp/info.el (Info-fontify-node):
* lisp/window.el (handle-select-window): Use display-graphic-p and
  display-multi-frame-p instead of explicit memq calls.
---
 lisp/disp-table.el | 14 ++++++-------
 lisp/faces.el      | 20 +++++++++----------
 lisp/frame.el      | 50 +++++++++++++++++++---------------------------
 lisp/info.el       |  2 +-
 lisp/window.el     |  4 +++-
 5 files changed, 40 insertions(+), 50 deletions(-)

diff --git a/lisp/disp-table.el b/lisp/disp-table.el
index 476c0cb986..4a59750677 100644
--- a/lisp/disp-table.el
+++ b/lisp/disp-table.el
@@ -175,8 +175,8 @@ standard-display-ascii
 (defun standard-display-g1 (c sc)
   "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))
+it is meaningless for a graphical frame."
+  (if (display-graphic-p)
       (error "Cannot use string glyphs in a windowing system"))
   (or standard-display-table
       (setq standard-display-table (make-display-table)))
@@ -186,9 +186,9 @@ standard-display-g1
 ;;;###autoload
 (defun standard-display-graphic (c gc)
   "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))
+This function assumes VT100-compatible escapes; it is meaningless
+for a graphical frame."
+  (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.
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.
diff --git a/lisp/frame.el b/lisp/frame.el
index 6cb1247372..eb4ab22c71 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-multi-frame-p frame)
     (x-focus-frame frame))
   ;; Move mouse cursor if necessary.
   (cond
@@ -1027,16 +1027,15 @@ 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))
-     ((eq type t)
-      (if (controlling-tty-p)
-	  (suspend-emacs)
-	(suspend-tty)))
-     (t (suspend-emacs)))))
+  (cond
+   ((display-multi-frame-p) (iconify-or-deiconify-frame))
+   ((eq (framep (selected-frame)) t)
+    (if (controlling-tty-p)
+        (suspend-emacs)
+      (suspend-tty)))
+   (t (suspend-emacs))))
 
 (defun make-frame-names-alist ()
   ;; Only consider the frames on the same display.
@@ -1933,12 +1932,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-multi-frame-p display)
+      (x-display-screens display)
+    1))
 
 (declare-function x-display-pixel-height "xfns.c" (&optional terminal))
 
@@ -1953,12 +1949,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 +1966,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 +2003,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 +2024,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))
diff --git a/lisp/info.el b/lisp/info.el
index f2a064abb6..f3b413a2f9 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))
+            (when (display-multi-font-p)
               (add-text-properties (1- (match-beginning 2)) (match-end 2)
                                    '(invisible t front-sticky nil rear-nonsticky t))))))
 
diff --git a/lisp/window.el b/lisp/window.el
index b769be0633..b4f5ac5cc4 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -9314,6 +9314,8 @@ mouse-autoselect-window-select
      ;; autoselection.
      (mouse-autoselect-window-start mouse-position window)))))
 
+(declare-function display-multi-frame-p "frame" (&optional display))
+
 (defun handle-select-window (event)
   "Handle select-window events."
   (interactive "^e")
@@ -9351,7 +9353,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 (not (display-multi-frame-p))
             (not focus-follows-mouse)
             ;; Focus FRAME if it's either a child frame or an ancestor
             ;; of the frame switched from.
-- 
2.21.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Define-and-use-new-procedure-display-symbol-keys-p.patch --]
[-- Type: text/x-patch, Size: 2553 bytes --]

From 964a107264a14ffbda1f29f81e1b9b6e1b6d4f35 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Wed, 3 Apr 2019 14:03:28 -0600
Subject: [PATCH 2/4] Define and use new procedure display-symbol-keys-p

* lisp/frame.el (display-symbol-keys-p): Define.

* lisp/simple.el (normal-erase-is-backspace-setup-frame): Use eq
instead of memq.
(normal-erase-is-backspace-mode): Use display-symbol-keys-p.
---
 lisp/frame.el  | 8 ++++++++
 lisp/simple.el | 7 ++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/lisp/frame.el b/lisp/frame.el
index eb4ab22c71..72b48c13de 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -1926,6 +1926,14 @@ display-selections-p
      (t
       nil))))
 
+(defun display-symbol-keys-p (&optional display)
+  "Return non-nil if DISPLAY supports symbol names as keys.
+This means that, for example, DISPLAY can differentiate between
+the keybinding RET and [return]."
+  (let ((frame-type (framep-on-display display)))
+    (or (memq frame-type '(x w32 ns pc))
+        (memq system-type '(ms-dos windows-nt)))))
+
 (declare-function x-display-screens "xfns.c" (&optional terminal))
 
 (defun display-screens (&optional display)
diff --git a/lisp/simple.el b/lisp/simple.el
index 306df96766..857e0fc001 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -8690,7 +8690,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
@@ -8701,6 +8701,8 @@ normal-erase-is-backspace-setup-frame
              normal-erase-is-backspace)
            1 0)))))
 
+(declare-function display-symbol-keys-p "frame" (&optional display))
+
 (define-minor-mode normal-erase-is-backspace-mode
   "Toggle the Erase and Delete mode of the Backspace and Delete keys.
 
@@ -8736,8 +8738,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))
-	       (memq system-type '(ms-dos windows-nt)))
+    (cond ((display-symbol-keys-p)
 	   (let ((bindings
 		  '(([M-delete] [M-backspace])
 		    ([C-M-delete] [C-M-backspace])
-- 
2.21.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-Define-and-use-new-alias-display-blink-cursor-p.patch --]
[-- Type: text/x-patch, Size: 1238 bytes --]

From 7e6a56a8ec549c9efc859184378a6619c1658c80 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Wed, 3 Apr 2019 14:03:42 -0600
Subject: [PATCH 3/4] Define and use new alias display-blink-cursor-p

display-graphic-p is not used in this case because it may be possible
in the future for terminals to allow control over cursor blinking. For
details, see bug#35058.

* lisp/frame.el (blink-cursor-mode): Use display-blink-cursor-p.
---
 lisp/frame.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/frame.el b/lisp/frame.el
index 72b48c13de..af89aecc3d 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -1905,6 +1905,7 @@ display-images-p
        (fboundp 'image-mask-p)
        (fboundp 'image-size)))
 
+(defalias 'display-blink-cursor-p 'display-graphic-p)
 (defalias 'display-multi-frame-p 'display-graphic-p)
 (defalias 'display-multi-font-p 'display-graphic-p)
 
@@ -2544,7 +2545,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-blink-cursor-p))))
   :initialize 'custom-initialize-delay
   :group 'cursor
   :global t
-- 
2.21.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0004-Introduce-new-defcustom-for-terminal-CUA-rectangle-c.patch --]
[-- Type: text/x-patch, Size: 2331 bytes --]

From 9e52e01e03c2c4cf14700527808f683ce86e5c58 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Wed, 3 Apr 2019 14:06:45 -0600
Subject: [PATCH 4/4] Introduce new defcustom for terminal CUA rectangle
 commands

This allows a user to set a non-meta modifier for their terminal
should his/her terminal support it. See bug#35058 for background on
this change.

* lisp/emulation/cua-base.el (cua-rectangle-terminal-modifier-key):
New defcustom.

* lisp/emulation/cua-base.el (cua--shift-control-x-prefix): Use new
defcustom.
---
 lisp/emulation/cua-base.el | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/lisp/emulation/cua-base.el b/lisp/emulation/cua-base.el
index 302ef12386..49e63c336f 100644
--- a/lisp/emulation/cua-base.el
+++ b/lisp/emulation/cua-base.el
@@ -427,7 +427,7 @@ cua-rectangle-mark-key
 
 (defcustom cua-rectangle-modifier-key 'meta
   "Modifier key used for rectangle commands bindings.
-On non-window systems, always use the meta modifier.
+On non-window systems, use `cua-rectangle-terminal-modifier-key'.
 Must be set prior to enabling CUA."
   :type '(choice (const :tag "Meta key" meta)
 		 (const :tag "Alt key" alt)
@@ -435,6 +435,16 @@ cua-rectangle-modifier-key
 		 (const :tag "Super key" super))
   :group 'cua)
 
+(defcustom cua-rectangle-terminal-modifier-key 'meta
+  "Modifier key used for rectangle commands bindings in terminals.
+Must be set prior to enabling CUA."
+  :type '(choice (const :tag "Meta key" meta)
+		 (const :tag "Alt key" alt)
+		 (const :tag "Hyper key" hyper)
+		 (const :tag "Super key" super))
+  :group 'cua
+  :version "27.1")
+
 (defcustom cua-enable-rectangle-auto-help t
   "If non-nil, automatically show help for region, rectangle and global mark."
   :type 'boolean
@@ -1237,10 +1247,9 @@ cua--shift-control-x-prefix
 (defun cua--init-keymaps ()
   ;; Cache actual rectangle modifier key.
   (setq cua--rectangle-modifier-key
-	(if (and cua-rectangle-modifier-key
-		 (memq window-system '(x)))
+	(if (eq (framep-on-display) t)
 	    cua-rectangle-modifier-key
-	  'meta))
+	  cua-rectangle-terminal-modifier-key))
   ;; C-return always toggles rectangle mark
   (define-key cua-global-keymap cua-rectangle-mark-key	'cua-set-rectangle-mark)
   (unless (eq cua--rectangle-modifier-key 'meta)
-- 
2.21.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #6: logb --]
[-- Type: text/x-patch, Size: 810 bytes --]

From 641cbc50d5d260a50075a2eb012ebd4ff021f5ee Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Tue, 2 Apr 2019 11:14:18 -0600
Subject: [PATCH] * lisp/frame.el (display-planes): Use logb over truncate +
 log

Suggested by Basil L. Contovounesios:
https://lists.gnu.org/archive/html/bug-gnu-emacs/2019-03/msg01052.html
---
 lisp/frame.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/frame.el b/lisp/frame.el
index 6cb1247372..5d8addf3c0 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2083,7 +2083,7 @@ display-planes
      ((eq frame-type 'pc)
       4)
      (t
-      (truncate (log (length (tty-color-alist)) 2))))))
+      (logb (length (tty-color-alist)))))))
 
 (declare-function x-display-color-cells "xfns.c" (&optional terminal))
 
-- 
2.21.0