* 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 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-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
[parent not found: <<83a7hagv11.fsf@gnu.org>]
* 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 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-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
[parent not found: <<8736n4ndav.fsf@gmail.com>]
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 external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.