* bug#35058: [PATCH] Use display-graphic-p in more cases
@ 2019-03-30 23:38 Alex
2019-03-31 12:45 ` Basil L. Contovounesios
2019-03-31 15:37 ` Eli Zaretskii
0 siblings, 2 replies; 19+ messages in thread
From: Alex @ 2019-03-30 23:38 UTC (permalink / raw)
To: 35058
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0001-Use-display-graphic-p-in-more-cases.patch --]
[-- Type: text/x-patch, Size: 14709 bytes --]
From b760955517f9c39b13353bdf5ae49f25fbadc66f Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Sat, 30 Mar 2019 16:53:11 -0600
Subject: [PATCH] Use display-graphic-p in more cases
Also, flip the logic so that adding new graphical window systems does
not need to touch as much code.
* lisp/frame.el:
* lisp/disp-table.el:
* lisp/faces.el: Use display-graphic-p instead of explicit memq calls.
* lisp/info.el (Info-fontify-node):
* lisp/window.el (handle-select-window): Flip logic of memq.
* lisp/simple.el (normal-erase-is-backspace-setup-frame):
* lisp/emulation/cua-base.el (cua--init-keymaps): Simplify
window-system checks.
---
lisp/disp-table.el | 8 +++---
lisp/emulation/cua-base.el | 2 +-
lisp/faces.el | 20 ++++++-------
lisp/frame.el | 59 ++++++++++++++++----------------------
lisp/info.el | 2 +-
lisp/simple.el | 4 +--
lisp/window.el | 2 +-
7 files changed, 43 insertions(+), 54 deletions(-)
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.
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
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..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))
;; Move mouse cursor if necessary.
(cond
@@ -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)
@@ -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))))
(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))
@@ -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))))
:initialize 'custom-initialize-delay
:group 'cursor
:global t
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))))))
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])
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.
--
2.21.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* bug#35058: [PATCH] Use display-graphic-p in more cases
2019-03-30 23:38 bug#35058: [PATCH] Use display-graphic-p in more cases Alex
@ 2019-03-31 12:45 ` Basil L. Contovounesios
2019-03-31 15:37 ` Eli Zaretskii
1 sibling, 0 replies; 19+ messages in thread
From: Basil L. Contovounesios @ 2019-03-31 12:45 UTC (permalink / raw)
To: Alex; +Cc: 35058
Just a minor comment from me.
Alex <agrambot@gmail.com> writes:
> diff --git a/lisp/frame.el b/lisp/frame.el
> index 6cb1247372..f5ad3152a0 100644
> --- a/lisp/frame.el
> +++ b/lisp/frame.el
> @@ -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))
I suggest also changing (truncate (log (length (tty-color-alist)) 2))
to (logb (length (tty-color-alist))).
Thanks,
--
Basil
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#35058: [PATCH] Use display-graphic-p in more cases
2019-03-30 23:38 bug#35058: [PATCH] Use display-graphic-p in more cases Alex
2019-03-31 12:45 ` Basil L. Contovounesios
@ 2019-03-31 15:37 ` Eli Zaretskii
2019-04-01 4:15 ` Alex
1 sibling, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2019-03-31 15:37 UTC (permalink / raw)
To: Alex; +Cc: 35058
> From: Alex <agrambot@gmail.com>
> Date: Sat, 30 Mar 2019 17:38:16 -0600
>
> >From b760955517f9c39b13353bdf5ae49f25fbadc66f Mon Sep 17 00:00:00 2001
> From: Alexander Gramiak <agrambot@gmail.com>
> 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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#35058: [PATCH] Use display-graphic-p in more cases
2019-03-31 15:37 ` Eli Zaretskii
@ 2019-04-01 4:15 ` Alex
2019-04-01 5:21 ` Eli Zaretskii
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Alex @ 2019-04-01 4:15 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 35058
Eli Zaretskii <eliz@gnu.org> 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?
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#35058: [PATCH] Use display-graphic-p in more cases
2019-04-01 4:15 ` Alex
@ 2019-04-01 5:21 ` Eli Zaretskii
2019-04-02 17:05 ` Alex
[not found] ` <<83a7hagv11.fsf@gnu.org>
2019-04-07 13:50 ` Stefan Monnier
2 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2019-04-01 5:21 UTC (permalink / raw)
To: Alex; +Cc: 35058
> 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. Let me tell more about
this.
These functions were written when TTY frames were made to support
faces (colors) during Emacs 21 development. When we started using
Emacs 21 on TTYs, we found out that various color-related features
didn't work since they were conditioned by window-system values,
because someone assumed that only GUI frames can ever support colors.
The easy solution was to remove the condition, but then it turned out
that many other places had code conditioned on window-system which had
nothing to do with colors or faces. So a simple removal was not
possible; moreover, careful auditing of each and every use of
window-system was needed to decide, on a per-case basis, which was and
which wasn't related to faces, something that wasn't always obvious,
because some conditions were added ad-hoc, to prevent code from
calling functions undefined in a build --without-x.
We then decided to factor all uses of window-system into several
classes and replace the uses of window-system by the proper predicates
whose _names_ will clearly tell what is the feature being tested.
That's when these display-* functions were written, and that's why the
use of window-system (the variable) was deprecated.
I would like to keep using these functions according to the above
logic, so that when we add support for new capabilities on non-GUI
frames, we will not need to hunt for tricky conditions all over the
code, not again.
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.
> >> 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.
See above: I could envision a feature that moves focus to a TTY frame
as well, some day.
> >> @@ -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?
The display (i.e. the terminal) could be a GUI one, but a frame it
shows could be a TTY frame, for example if you use a terminal emulator
such as xterm. We all do that all the time.
> If there is indeed no logical relation, then what about a new
> display-iconifiable-p that is made an alias?
As I said above, I consider that slightly gross, but I won't object to
introducing such functions.
> > 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.
I hope you now understand my motivation better. IME, deciphering
those "few" definitions is not really trivial, especially when they
are used slightly inconsistently through the sources. Making the
conditions explicit goes a small step in the direction of making that
job easier.
> 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."
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.
> >> ;; 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.
> > 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.
> >> 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).
Thanks.
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#35058: [PATCH] Use display-graphic-p in more cases
[not found] ` <<83a7hagv11.fsf@gnu.org>
@ 2019-04-01 14:32 ` Drew Adams
2019-04-06 7:18 ` Eli Zaretskii
0 siblings, 1 reply; 19+ messages in thread
From: Drew Adams @ 2019-04-01 14:32 UTC (permalink / raw)
To: Eli Zaretskii, Alex; +Cc: 35058
This is a great description, Eli.
I hope it or similar is recorded somewhere in
code or code-related artifacts, not just in
mail threads.
It can also help users to know some of this
(the rationale). That isn't so important now
perhaps, as things like `display-multi-font-p'
are just aliased. But it could be more
important later.
BTW, where would the intended meaning of such
functions be recorded now? Currently their
doc is just the doc of `display-graphic-p',
as they are aliased. (Could/should they have
their own doc strings now, even if the behavior
is currently just `display-graphic-p'?)
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#35058: [PATCH] Use display-graphic-p in more cases
2019-04-01 5:21 ` Eli Zaretskii
@ 2019-04-02 17:05 ` Alex
2019-04-02 17:23 ` Eli Zaretskii
0 siblings, 1 reply; 19+ messages in thread
From: Alex @ 2019-04-02 17:05 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 35058
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?
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#35058: [PATCH] Use display-graphic-p in more cases
2019-04-02 17:05 ` Alex
@ 2019-04-02 17:23 ` Eli Zaretskii
2019-04-02 17:57 ` Alex
0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2019-04-02 17:23 UTC (permalink / raw)
To: Alex; +Cc: 35058
> From: Alex <agrambot@gmail.com>
> Cc: 35058@debbugs.gnu.org
> Date: Tue, 02 Apr 2019 11:05:09 -0600
>
> >> 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?
I don't know. What do you mean by "accept"?
> Should I remove the window-system condition even if I can't get
> terminal Emacs to recognize the hyper key?
If it is very inconvenient to use hyper on text terminals, then I
think we shouldn't require users to do that.
> >> 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?
How will you know which tests to remove? If the predicate was named
display-blink-cursor-p, then you'd know immediately, but if the
predicate is display-graphic-p, you'd need to decide which of those to
remove and which not to remove. These predicates exist to make the
decision easy.
> 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?
You mean, C-m/RET on the one hand and [return] on the other, right? I
think a new predicate could be beneficial, because the function keys
are supported on some text terminals, for example on MS-Windows. How
about display-function-keys-p?
> > 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.
display-can-raise-frames-p or something. (But I'm not good with
coming up with names.)
> What about using display-multi-frame-p? Could there be some system
> that allows multiple frames, but no way to select between them?
Text terminals can support multiple frames, so display-multi-frame-p
is not what we want here.
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#35058: [PATCH] Use display-graphic-p in more cases
2019-04-02 17:23 ` Eli Zaretskii
@ 2019-04-02 17:57 ` Alex
2019-04-02 18:39 ` Eli Zaretskii
0 siblings, 1 reply; 19+ messages in thread
From: Alex @ 2019-04-02 17:57 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 35058
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Alex <agrambot@gmail.com>
>> Cc: 35058@debbugs.gnu.org
>> Date: Tue, 02 Apr 2019 11:05:09 -0600
>>
>> >> 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?
>
> I don't know. What do you mean by "accept"?
I can get terminal Emacs to recognize the super modifier (it shows up in
C-h l as a `s-' prefix), but not the hyper modifier (all input is
treated as if it wasn't used at all). Does the hyper modifier work for
you in the terminal?
In my GTK Emacs both modifiers work as expected.
>> Should I remove the window-system condition even if I can't get
>> terminal Emacs to recognize the hyper key?
>
> 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.
>> >> 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?
>
> How will you know which tests to remove? If the predicate was named
> display-blink-cursor-p, then you'd know immediately, but if the
> predicate is display-graphic-p, you'd need to decide which of those to
> remove and which not to remove. These predicates exist to make the
> decision easy.
In this case there is only one test to remove, which is clearly
mentioned in the docstring. I suppose my point before about 3rd-party
code would apply here too, so adding display-blink-cursor-p would make
sense.
>> 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?
>
> You mean, C-m/RET on the one hand and [return] on the other, right? I
> think a new predicate could be beneficial, because the function keys
> are supported on some text terminals, for example on MS-Windows. How
> about display-function-keys-p?
[return] is another example, but I believe C-m and RET can be different
in graphical Emacs. In my config I do:
(define-key input-decode-map "\C-m" [C-m])
This allows me to use the control modifier with m separate from both RET
and [return], as `C-h c' on the return key outputs:
RET (translated from <return>) runs the command newline
However, doing the same in terminal Emacs makes the return key use my
custom [C-m] prefix. This is an example of the behaviour the predicate
should be covering.
I'm not sure about display-function-keys-p. That would seem to imply
behaviour surrounding the `Fn' key on many keyboards.
I'm not good with names either, but if we can't find a good specific
name, then what about a more generic name like
display-full-keybindings-p or display-extra-keybindings-p where the
docstring would describe the behaviour more thoroughly?
>> > 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.
>
> display-can-raise-frames-p or something. (But I'm not good with
> coming up with names.)
Not sure, perhaps display-can-change-focus-p?
>> What about using display-multi-frame-p? Could there be some system
>> that allows multiple frames, but no way to select between them?
>
> Text terminals can support multiple frames, so display-multi-frame-p
> is not what we want here.
If that's the case, then why is display-multi-frame-p currently an alias
for display-graphic-p?
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#35058: [PATCH] Use display-graphic-p in more cases
2019-04-02 17:57 ` Alex
@ 2019-04-02 18:39 ` Eli Zaretskii
2019-04-03 5:14 ` Alex
0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2019-04-02 18:39 UTC (permalink / raw)
To: Alex; +Cc: 35058
> From: Alex <agrambot@gmail.com>
> Cc: 35058@debbugs.gnu.org
> Date: Tue, 02 Apr 2019 11:57:50 -0600
>
> >> Hmm, it seems that my terminal Emacs accepts the super modifier but not
> >> the hyper modifier; is this a bug in Emacs?
> >
> > I don't know. What do you mean by "accept"?
>
> I can get terminal Emacs to recognize the super modifier (it shows up in
> C-h l as a `s-' prefix), but not the hyper modifier (all input is
> treated as if it wasn't used at all).
That might mean your terminal is unable to _generate_ the modifier.
What happens if you use the "C-x @ h" prefix, does it produce
hyper-modified keys?
> > 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?
> > You mean, C-m/RET on the one hand and [return] on the other, right? I
> > think a new predicate could be beneficial, because the function keys
> > are supported on some text terminals, for example on MS-Windows. How
> > about display-function-keys-p?
>
> [return] is another example, but I believe C-m and RET can be different
> in graphical Emacs. In my config I do:
>
> (define-key input-decode-map "\C-m" [C-m])
>
> This allows me to use the control modifier with m separate from both RET
> and [return], as `C-h c' on the return key outputs:
>
> RET (translated from <return>) runs the command newline
>
> However, doing the same in terminal Emacs makes the return key use my
> custom [C-m] prefix. This is an example of the behaviour the predicate
> should be covering.
I think [return] the function key is a much more frequent case for the
distinction. TTY frames generally don't have function keys as
symbols, they have escape sequences which we translate to function
keys.
> I'm not sure about display-function-keys-p. That would seem to imply
> behaviour surrounding the `Fn' key on many keyboards.
A doc string will explain what we mean.
> >> What about using display-multi-frame-p? Could there be some system
> >> that allows multiple frames, but no way to select between them?
> >
> > Text terminals can support multiple frames, so display-multi-frame-p
> > is not what we want here.
>
> If that's the case, then why is display-multi-frame-p currently an alias
> for display-graphic-p?
I don't remember. But maybe I'm splitting hair here, and we can use
this predicate for iconifying etc. as well.
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#35058: [PATCH] Use display-graphic-p in more cases
2019-04-02 18:39 ` Eli Zaretskii
@ 2019-04-03 5:14 ` Alex
2019-04-03 5:29 ` Eli Zaretskii
0 siblings, 1 reply; 19+ messages in thread
From: Alex @ 2019-04-03 5:14 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 35058
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Alex <agrambot@gmail.com>
>> Cc: 35058@debbugs.gnu.org
>> Date: Tue, 02 Apr 2019 11:57:50 -0600
>>
>> >> Hmm, it seems that my terminal Emacs accepts the super modifier but not
>> >> the hyper modifier; is this a bug in Emacs?
>> >
>> > I don't know. What do you mean by "accept"?
>>
>> I can get terminal Emacs to recognize the super modifier (it shows up in
>> C-h l as a `s-' prefix), but not the hyper modifier (all input is
>> treated as if it wasn't used at all).
>
> That might mean your terminal is unable to _generate_ the modifier.
> What happens if you use the "C-x @ h" prefix, does it produce
> hyper-modified keys?
You're right, and it does work with "C-x @ h". It seems that terminals
supporting either super/hyper are rare (or nonexistent -- I tried a few
popular ones to no avail). My terminal only happens to support super
since someone hard-coded it to output "C-x @ s" with the super modifier
specifically to support Emacs.
>> > 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.
>> > You mean, C-m/RET on the one hand and [return] on the other, right? I
>> > think a new predicate could be beneficial, because the function keys
>> > are supported on some text terminals, for example on MS-Windows. How
>> > about display-function-keys-p?
>>
>> [return] is another example, but I believe C-m and RET can be different
>> in graphical Emacs. In my config I do:
>>
>> (define-key input-decode-map "\C-m" [C-m])
>>
>> This allows me to use the control modifier with m separate from both RET
>> and [return], as `C-h c' on the return key outputs:
>>
>> RET (translated from <return>) runs the command newline
>>
>> However, doing the same in terminal Emacs makes the return key use my
>> custom [C-m] prefix. This is an example of the behaviour the predicate
>> should be covering.
>
> I think [return] the function key is a much more frequent case for the
> distinction. TTY frames generally don't have function keys as
> symbols, they have escape sequences which we translate to function
> keys.
Yeah, I'll agree that [return] is more common; I was just describing my
usage of C-m (or [C-m]) here.
Why do you only say that TTY frames _generally_ don't have function
keys as symbols?
>> I'm not sure about display-function-keys-p. That would seem to imply
>> behaviour surrounding the `Fn' key on many keyboards.
>
> A doc string will explain what we mean.
Right, but I still don't think the name is particularly great
considering the common notion of function key
(en.wikipedia.org/wiki/Function_key).
How about display-symbol-keys-p (if TTY frames can't support symbols in
key sequences)?
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#35058: [PATCH] Use display-graphic-p in more cases
2019-04-03 5:14 ` Alex
@ 2019-04-03 5:29 ` Eli Zaretskii
2019-04-03 20:26 ` Alex
0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2019-04-03 5:29 UTC (permalink / raw)
To: Alex; +Cc: 35058
> 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.
> Why do you only say that TTY frames _generally_ don't have function
> keys as symbols?
Because there are exceptions, like the MS-Windows console.
> 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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#35058: [PATCH] Use display-graphic-p in more cases
2019-04-03 5:29 ` Eli Zaretskii
@ 2019-04-03 20:26 ` Alex
2019-04-05 7:29 ` Eli Zaretskii
0 siblings, 1 reply; 19+ messages in thread
From: Alex @ 2019-04-03 20:26 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 35058
[-- 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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* bug#35058: [PATCH] Use display-graphic-p in more cases
2019-04-03 20:26 ` Alex
@ 2019-04-05 7:29 ` Eli Zaretskii
2019-04-05 16:35 ` Alex
0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2019-04-05 7:29 UTC (permalink / raw)
To: Alex; +Cc: 35058
> From: Alex <agrambot@gmail.com>
> Cc: 35058@debbugs.gnu.org
> Date: Wed, 03 Apr 2019 14:26:30 -0600
>
> I've attached a patch series below (including the logb change Basil
> recommended). Are these okay to apply?
Almost, we are close. See the remaining few comments below.
> 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?
For the display-* functions, yes, I'd like to leave their definitions
explicit.
> +(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)))))
Please add a comment here explaining that MS-Windows/MS-DOS terminals
are an exception because they have built-in support for function keys.
> --- 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))
Did you try to bootstrap with this? frame.el is loaded after
faces.el. (There are more similar declarations in the patches with
the same issues.)
> --- 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")
This change should be called out in NEWS.
The new display-* predicates should probably also be mentioned in
NEWS.
Thanks.
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#35058: [PATCH] Use display-graphic-p in more cases
2019-04-05 7:29 ` Eli Zaretskii
@ 2019-04-05 16:35 ` Alex
2019-04-05 18:51 ` Eli Zaretskii
0 siblings, 1 reply; 19+ messages in thread
From: Alex @ 2019-04-05 16:35 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 35058
[-- Attachment #1: Type: text/plain, Size: 1821 bytes --]
Eli Zaretskii <eliz@gnu.org> writes:
>> --- 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))
>
> Did you try to bootstrap with this? frame.el is loaded after
> faces.el. (There are more similar declarations in the patches with
> the same issues.)
Yes, I successfully bootstrapped, and the loading issue is why I added
the declare-function calls. It seems to work fine. My usage of
display-graphic-p appears to be different compared to the one in
face-spec-reset-face, which apparently is run during bootstrap.
>> --- 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")
>
> This change should be called out in NEWS.
>
> The new display-* predicates should probably also be mentioned in
> NEWS.
Okay, here's a new set of patches (excluding the same logb patch).
[-- 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: 8507 bytes --]
From 3f476d5e827fc1e94a18101604838923a2a686c7 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 | 19 +++++++++----------
lisp/info.el | 2 +-
lisp/window.el | 4 +++-
5 files changed, 29 insertions(+), 30 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..acf6a46716 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.
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-alias-display-blink-cursor-p.patch --]
[-- Type: text/x-patch, Size: 1238 bytes --]
From 372299922bbff28d8fe19fffc6c784b390e13402 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Wed, 3 Apr 2019 14:03:42 -0600
Subject: [PATCH 2/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 acf6a46716..cc8ca49b3b 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)
@@ -2545,7 +2546,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 #4: 0003-Define-and-use-new-procedure-display-symbol-keys-p.patch --]
[-- Type: text/x-patch, Size: 3255 bytes --]
From 6d171ce90f42f16f148fae87f083038cc95acf05 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Wed, 3 Apr 2019 14:03:28 -0600
Subject: [PATCH 3/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.
---
etc/NEWS | 5 +++++
lisp/frame.el | 10 ++++++++++
lisp/simple.el | 7 ++++---
3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/etc/NEWS b/etc/NEWS
index 7f6aeab73f..1e64c0f15f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1226,6 +1226,11 @@ the 128...255 range, as expected.
This allows to create and parent immediately a minibuffer-only child
frame when making a frame.
+*** New predicates 'display-blink-cursor-p' and 'display-symbol-keys-p'.
+These predicates are to be preferred over 'display-graphic-p' when
+testing for blinking cursor capability and the capability to have
+symbols (e.g., [return], [tab], [backspace]) as keys respectively.
+
** Tabulated List mode
+++
diff --git a/lisp/frame.el b/lisp/frame.el
index cc8ca49b3b..aa14e87d7b 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -1927,6 +1927,16 @@ 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))
+ ;; MS-DOS and MS-Windows terminals have built-in support for
+ ;; function (symbol) keys
+ (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 #5: 0004-Introduce-new-defcustom-for-terminal-CUA-rectangle-c.patch --]
[-- Type: text/x-patch, Size: 2882 bytes --]
From 04766b452e895526f2ca196dad7cf05bab1e61ba 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.
---
etc/NEWS | 6 ++++++
lisp/emulation/cua-base.el | 19 ++++++++++++++-----
2 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/etc/NEWS b/etc/NEWS
index 1e64c0f15f..5c62daf2b9 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1247,6 +1247,12 @@ near the current column in Tabulated Lists (see variables
+++
*** 'text-mode-variant' is now obsolete, use 'derived-mode-p' instead.
+** CUA mode
+
+*** New defcustom 'cua-rectangle-terminal-modifier-key'
+This defcustom is used instead of forcing the modifier key to
+'meta in a terminal frame.
+
\f
* New Modes and Packages in Emacs 27.1
diff --git a/lisp/emulation/cua-base.el b/lisp/emulation/cua-base.el
index 302ef12386..105e1ab43d 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)))
- cua-rectangle-modifier-key
- 'meta))
+ (if (eq (framep (selected-frame)) t)
+ cua-rectangle-terminal-modifier-key
+ cua-rectangle-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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* bug#35058: [PATCH] Use display-graphic-p in more cases
2019-04-05 16:35 ` Alex
@ 2019-04-05 18:51 ` Eli Zaretskii
2019-04-07 5:11 ` Alex
0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2019-04-05 18:51 UTC (permalink / raw)
To: Alex; +Cc: 35058
> From: Alex <agrambot@gmail.com>
> Cc: 35058@debbugs.gnu.org
> Date: Fri, 05 Apr 2019 10:35:33 -0600
>
> Okay, here's a new set of patches (excluding the same logb patch).
Thanks, this LGTM, with a couple of nits:
> +** CUA mode
> +
> +*** New defcustom 'cua-rectangle-terminal-modifier-key'
Please add a period at the end of this sentence.
> +This defcustom is used instead of forcing the modifier key to
I'd say "allows to customize the modifier key ..." instead of "is used
instead of ...".
Also, please mark this entry with "---", as it doesn't need to be
documented in any manuals. (The other NEWS entry as well.)
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#35058: [PATCH] Use display-graphic-p in more cases
2019-04-01 14:32 ` Drew Adams
@ 2019-04-06 7:18 ` Eli Zaretskii
0 siblings, 0 replies; 19+ messages in thread
From: Eli Zaretskii @ 2019-04-06 7:18 UTC (permalink / raw)
To: Drew Adams; +Cc: 35058, agrambot
> Date: Mon, 1 Apr 2019 07:32:23 -0700 (PDT)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: 35058@debbugs.gnu.org
>
> This is a great description, Eli.
>
> I hope it or similar is recorded somewhere in
> code or code-related artifacts, not just in
> mail threads.
Thanks, I added commentary to frame.el, where these functions are
defined.
> BTW, where would the intended meaning of such
> functions be recorded now? Currently their
> doc is just the doc of `display-graphic-p',
> as they are aliased. (Could/should they have
> their own doc strings now, even if the behavior
> is currently just `display-graphic-p'?)
I don't think we have features to have separate doc strings for them,
I hope the commentary I wrote is enough.
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#35058: [PATCH] Use display-graphic-p in more cases
2019-04-05 18:51 ` Eli Zaretskii
@ 2019-04-07 5:11 ` Alex
0 siblings, 0 replies; 19+ messages in thread
From: Alex @ 2019-04-07 5:11 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 35058
close 35058
quit
Okay, pushed as 8f6e479845..0c16bb5a39.
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#35058: [PATCH] Use display-graphic-p in more cases
2019-04-01 4:15 ` Alex
2019-04-01 5:21 ` Eli Zaretskii
[not found] ` <<83a7hagv11.fsf@gnu.org>
@ 2019-04-07 13:50 ` Stefan Monnier
2 siblings, 0 replies; 19+ messages in thread
From: Stefan Monnier @ 2019-04-07 13:50 UTC (permalink / raw)
To: Alex; +Cc: 35058
> 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?
IIUC the idea of this var is that many of the key-bindings cua-rectangle
uses (when not using the meta modifier) are not properly handled by the
vast majority of text-terminals (e.g. hitting C-RET will send Emacs the
same events as just hitting RET). The meta modifier (which can
be accessed as an ESC prefix when needed) doesn't suffer from the
same problem.
>>> - (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.
Arguably, the test should be removed and the function called
unconditionally (or if it's not always defined, then the test should be
(fboundp 'x-focus-frame)).
> 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."
I think this description is incorrect. blink-cursor-mode works just
fine (under xterm at least), with the patch below.
But I think this is not just a case of "it doesn't work":
cursor-blinking can be configured in xterm independently from the
application, so Emacs likely should just obey that choice, by default
(whereas for GUI frames, Emacs is the only one who can decide it).
Stefan
diff --git a/lisp/frame.el b/lisp/frame.el
index 6cb1247372..c7fe316c70 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -26,6 +26,7 @@
;;; Code:
(eval-when-compile (require 'cl-lib))
+(eval-when-compile (require 'subr-x)) ;For string-trim-right
(cl-defgeneric frame-creation-function (params)
"Method for window-system dependent functions to create a new frame.
@@ -2480,14 +2481,34 @@ blink-cursor-timer-function
(when (and (> blink-cursor-blinks 0)
(<= (* 2 blink-cursor-blinks) blink-cursor-blinks-done))
(blink-cursor-suspend)
- (add-hook 'post-command-hook 'blink-cursor-check)))
+ (add-hook 'post-command-hook #'blink-cursor-check))
+ ;; FIXME: Under TTYs, apparently redisplay only obeys internal-show-cursor
+ ;; when there is something else to update on the screen. This is arguably
+ ;; a bug, but in the meantime we can circumvent it here by causing an
+ ;; artificial update which thus "forces" a cursor update.
+ (when (null window-system)
+ (let* ((message-log-max nil)
+ (msg (current-message))
+ ;; Construct a dummy temp message different from the current one.
+ ;; This message usually flashes by too quickly to be visible, but
+ ;; occasionally it can be noticed, so make it "inconspicuous".
+ ;; Not too "inconspicuous", tho: just adding or removing a SPC at the
+ ;; end doesn't cause an update, for example.
+ (dummymsg (concat (if (> (length msg) 40)
+ (let ((msg (string-trim-right msg)))
+ (if (> (length msg) 2)
+ (substring msg 0 -2)
+ msg))
+ msg) "-")))
+ (message "%s" dummymsg)
+ (if msg (message "%s" msg) (message nil)))))
(defun blink-cursor-end ()
"Stop cursor blinking.
This is installed as a pre-command hook by `blink-cursor-start'.
When run, it cancels the timer `blink-cursor-timer' and removes
itself as a pre-command hook."
- (remove-hook 'pre-command-hook 'blink-cursor-end)
+ (remove-hook 'pre-command-hook #'blink-cursor-end)
(internal-show-cursor nil t)
(when blink-cursor-timer
(cancel-timer blink-cursor-timer)
@@ -2506,15 +2527,7 @@ blink-cursor-suspend
(defun blink-cursor--should-blink ()
"Determine whether we should be blinking.
Returns whether we have any focused non-TTY frame."
- (and blink-cursor-mode
- (let ((frame-list (frame-list))
- (any-graphical-focused nil))
- (while frame-list
- (let ((frame (pop frame-list)))
- (when (and (display-graphic-p frame) (frame-focus-state frame))
- (setf any-graphical-focused t)
- (setf frame-list nil))))
- any-graphical-focused)))
+ blink-cursor-mode)
(defun blink-cursor-check ()
"Check if cursor blinking shall be restarted.
@@ -2523,7 +2536,7 @@ blink-cursor-check
`blink-cursor--should-blink' and returns its result."
(let ((should-blink (blink-cursor--should-blink)))
(when (and should-blink (not blink-cursor-idle-timer))
- (remove-hook 'post-command-hook 'blink-cursor-check)
+ (remove-hook 'post-command-hook #'blink-cursor-check)
(blink-cursor--start-idle-timer))
should-blink))
^ permalink raw reply related [flat|nested] 19+ messages in thread
end of thread, other threads:[~2019-04-07 13:50 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-30 23:38 bug#35058: [PATCH] Use display-graphic-p in more cases Alex
2019-03-31 12:45 ` Basil L. Contovounesios
2019-03-31 15:37 ` Eli Zaretskii
2019-04-01 4:15 ` Alex
2019-04-01 5:21 ` Eli Zaretskii
2019-04-02 17:05 ` Alex
2019-04-02 17:23 ` Eli Zaretskii
2019-04-02 17:57 ` Alex
2019-04-02 18:39 ` Eli Zaretskii
2019-04-03 5:14 ` Alex
2019-04-03 5:29 ` Eli Zaretskii
2019-04-03 20:26 ` Alex
2019-04-05 7:29 ` Eli Zaretskii
2019-04-05 16:35 ` Alex
2019-04-05 18:51 ` Eli Zaretskii
2019-04-07 5:11 ` Alex
[not found] ` <<83a7hagv11.fsf@gnu.org>
2019-04-01 14:32 ` Drew Adams
2019-04-06 7:18 ` Eli Zaretskii
2019-04-07 13:50 ` Stefan Monnier
[not found] <<8736n4ndav.fsf@gmail.com>
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).