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

[Stefan Monnier <monnier@IRO.UMontreal.CA>]

> 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))