unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#54826: 29.0.50; Prevent duplicate prompts when reconnecting in ERC
@ 2022-04-09 20:29 J.P.
  0 siblings, 0 replies; 3+ messages in thread
From: J.P. @ 2022-04-09 20:29 UTC (permalink / raw)
  To: 54826; +Cc: emacs-erc

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

Tags: patch

Currently, when a user disconnects from an IRC session, the input prompt
in the server buffer vanishes. That probably wouldn't matter if the
primary means of reconnecting didn't involve needing that very prompt.
That is, the recommended way to reconnect is indeed to issue a
/reconnect "slash-command" in the input area (the newly nondescript
region preceding EOB). IMO, this degrades the overall user experience
because a user needs prior knowledge (or must magically intuit) that the
input area is in fact still viable and responsive.

But that's not all. As soon as that command is issued (normally via
RET), a second prompt appears before connection activities commence.
Depending on a user's setup and the endpoint they're connecting to, a
few things can happen. But most often, server messages start appearing
between the two prompts.

The proposed patch leverages a long abandoned option called
`erc-hide-prompt' to help address both these concerns. Revived and
repurposed, it now holds an indicator of the desired prompt behavior
upon disconnecting. By default, it merely "hides" the prompt by
replacing it with a user-configurable string (currently ">" by default).
This happens in all buffers owned by the connection, including target
buffers. When the value is 'server' or 'target', only those buffer types
are affected. When the value is nil, prompts are never hidden. Hidden
prompts can be revealed by typing in the input area. However, once
revealed, a prompt won't be rehidden (until the next disconnection).

Questions:

1. Currently, channels and queries are lumped together as 'target'. What
   about changing the shape of the option to a set of {'server',
   'channel', 'query'} in a union with t for "all of the above" (to
   preserve compatibility)?

2. Currently, when the value of `erc-hide-prompt' is 'target' or t,
   prompts in channels are automatically revealed upon reconnecting, but
   in queries, they aren't. This adheres faithfully to the protocol and
   accurately reflects the internal state of the model. Broadly
   speaking, a client can't reliably tell whether a party they're
   corresponding with in a query is still available (unless they share a
   channel).

   Is overloading the "prompt hidden" indicator with multiple meanings
   too distracting? (Currently, it means both "disconnected" and
   "physically connected but possibly not resumable".) Should we instead
   just reveal the prompt for all queries upon reconnecting, even though
   it may not be possible to continue certain conversations? Would it be
   worth accommodating both behaviors via yet another knob (or perhaps
   different 'query' variants, when going with 1, above)?

2. Right now, the prompt looks like this when hidden:

   n-1 | *** ERC finished ***                                   [04:30]
   n   | >_

   Where _ is the cursor at point max. Should we leave a space after the
   ">"?

This particular patch may benefit from integration trials with popular
external ERC modules as well as exotic values for various built-in
options. Any takers, please be aware that ERC famously has trouble
reassociating buffers upon reconnecting; if that's likely to interfere
with your testing, please consider trying #48598 (which currently
contains this patch). Volunteers (and detractors) welcome. Thanks.


In GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.31, cairo version 1.17.4)
 of 2022-04-05 built on localhost
Repository revision: e2fb5ecaea67497224455fdbfe4850a5a74c9d00
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12014000
System Description: Fedora Linux 35 (Workstation Edition)

Configured using:
 'configure --enable-check-lisp-object-type --enable-checking=yes,glyphs
 'CFLAGS=-O0 -g3'
 PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media rmc puny
dired dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg
rfc6068 epg-config gnus-util text-property-search time-date seq gv
subr-x byte-opt bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
iso-transl tooltip eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/x-win x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer nadvice
simple cl-generic cham georgian utf-8-lang misc-lang vietnamese tibetan
thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian
slovak czech european ethiopic indian cyrillic chinese composite
emoji-zwj charscript charprop case-table epa-hook jka-cmpr-hook help
abbrev obarray oclosure cl-preloaded button loaddefs faces cus-face
macroexp files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting cairo move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 43818 5691)
 (symbols 48 5713 1)
 (strings 32 15808 1645)
 (string-bytes 1 527821)
 (vectors 16 12098)
 (vector-slots 8 168912 12189)
 (floats 8 20 34)
 (intervals 56 221 0)
 (buffers 992 10))

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-some-ERC-test-helpers.patch --]
[-- Type: text/x-patch, Size: 2161 bytes --]

From 2cd0a9e768859bdb3218233bffe277b53840a56d Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Tue, 5 Apr 2022 17:45:00 -0700
Subject: [PATCH 1/2] Add some ERC test helpers

* test/lisp/erc/erc-tests.el (erc-tests--test-prep,
erc-tests--set-fake-server-process): Factor out some common
buffer-prep boilerplate involving user input and the server process.
Shared with bug#54536.
---
 test/lisp/erc/erc-tests.el | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 520f10dd4e..373bed55af 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -114,6 +114,20 @@ erc-with-all-buffers-of-server
     (should (get-buffer "#spam"))
     (kill-buffer "#spam")))
 
+(defun erc-tests--send-prep ()
+  (erc-mode)
+  (insert "\n\n")
+  (setq erc-input-marker (make-marker)
+        erc-insert-marker (make-marker))
+  (set-marker erc-insert-marker (point-max))
+  (erc-display-prompt)
+  (should (= (point) erc-input-marker)))
+
+(defun erc-tests--set-fake-server-process (&rest args)
+  (setq erc-server-process
+        (apply #'start-process (car args) (current-buffer) args))
+  (set-process-query-on-exit-flag erc-server-process nil))
+
 (ert-deftest erc--switch-to-buffer ()
   (defvar erc-modified-channels-alist) ; lisp/erc/erc-track.el
 
@@ -197,14 +211,10 @@ erc-ring-previous-command-base-case
 (ert-deftest erc-ring-previous-command ()
   (with-current-buffer (get-buffer-create "*#fake*")
     (erc-mode)
-    (insert "\n\n")
+    (erc-tests--send-prep)
+    (setq-local erc-last-input-time 0)
     (should-not (local-variable-if-set-p 'erc-send-completed-hook))
     (set (make-local-variable 'erc-send-completed-hook) nil) ; skip t (globals)
-    (setq erc-input-marker (make-marker)
-          erc-insert-marker (make-marker))
-    (set-marker erc-insert-marker (point-max))
-    (erc-display-prompt)
-    (should (= (point) erc-input-marker))
     ;; Just in case erc-ring-mode is already on
     (setq-local erc-pre-send-functions nil)
     (add-hook 'erc-pre-send-functions #'erc-add-to-input-ring)
-- 
2.35.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-SQUASH-ME-Remove-duplicate-ERC-prompt-on-reconnect.patch --]
[-- Type: text/x-patch, Size: 12431 bytes --]

From 500e33853f597da47a8e2ad23b1e438373058d70 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Mon, 4 Apr 2022 01:50:50 -0700
Subject: [PATCH 2/2] [SQUASH-ME] Remove duplicate ERC prompt on reconnect

* lisp/erc/erc-backend.el (erc--unhide-prompt, erc--hide-prompt,
erc--unhide-prompt-on-self-insert): Add functions to ensure prompt is
hidden on disconnect and shown when a user types /reconnect in a
disconnected server buffer.
(erc-process-sentinel): Register aforementioned function with
`pre-command-hook' when prompt is deleted after disconnecting.
(erc-server-PRIVMSG): ensure prompt is showing when a new message
arrives from target.

* lisp/erc/erc.el (erc-hide-prompt): Repurpose unused option by
changing meaning slightly to mean "selectively hide prompt when
disconnected."  Also delete obsolete, commented-out code that at some
point used this option in its prior incarnation.
(erc-prompt-hidden): Add new option to specify look of prompt when
hidden.
(erc-open): Augment earlier reconnect-detection
semantics by incorporating `erc--server-reconnecting'.  In existing
buffers, remove prompt-related hooks and reveal prompt, if necessary.
(erc-cmd-RECONNECT): allow a user to reconnect when already
connected (by first disconnecting).
---
 lisp/erc/erc-backend.el    | 35 +++++++++++++---
 lisp/erc/erc.el            | 56 +++++++++++--------------
 test/lisp/erc/erc-tests.el | 85 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 138 insertions(+), 38 deletions(-)

diff --git a/lisp/erc/erc-backend.el b/lisp/erc/erc-backend.el
index 398fe6cc9e..593ee735f5 100644
--- a/lisp/erc/erc-backend.el
+++ b/lisp/erc/erc-backend.el
@@ -696,6 +696,33 @@ erc-process-sentinel-1
       ;; unexpected disconnect
       (erc-process-sentinel-2 event buffer))))
 
+(defun erc--unhide-prompt ()
+  (remove-hook 'pre-command-hook #'erc--unhide-prompt-on-self-insert t)
+  (when (and (marker-position erc-insert-marker)
+             (marker-position erc-input-marker))
+    (with-silent-modifications
+      (remove-text-properties erc-insert-marker erc-input-marker
+                              '(display nil)))))
+
+(defun erc--unhide-prompt-on-self-insert ()
+  (when (and (eq this-command #'self-insert-command)
+             (or (eobp) (= (point) erc-input-marker)))
+    (erc--unhide-prompt)))
+
+(defun erc--hide-prompt (proc)
+  (erc-with-all-buffers-of-server
+      proc nil ; sorta wish this was indent 2
+      (when (and erc-hide-prompt
+                 (memq erc-hide-prompt
+                       (list t (if (erc-default-target) 'target 'server)))
+                 (marker-position erc-insert-marker)
+                 (marker-position erc-input-marker)
+                 (get-text-property erc-insert-marker 'erc-prompt))
+        (with-silent-modifications
+          (add-text-properties erc-insert-marker
+                               erc-input-marker `(display ,erc-prompt-hidden)))
+        (add-hook 'pre-command-hook #'erc--unhide-prompt-on-self-insert 0 t))))
+
 (defun erc-process-sentinel (cproc event)
   "Sentinel function for ERC process."
   (let ((buf (process-buffer cproc)))
@@ -718,11 +745,8 @@ erc-process-sentinel
           (dolist (buf (erc-buffer-filter (lambda () (boundp 'erc-channel-users)) cproc))
             (with-current-buffer buf
               (setq erc-channel-users (make-hash-table :test 'equal))))
-          ;; Remove the prompt
-          (goto-char (or (marker-position erc-input-marker) (point-max)))
-          (forward-line 0)
-          (erc-remove-text-properties-region (point) (point-max))
-          (delete-region (point) (point-max))
+          ;; Hide the prompt
+          (erc--hide-prompt cproc)
           ;; Decide what to do with the buffer
           ;; Restart if disconnected
           (erc-process-sentinel-1 event buf))))))
@@ -1470,6 +1494,7 @@ define-erc-response-handler
         (setq buffer (erc-get-buffer (if privp nick tgt) proc))
         (when buffer
           (with-current-buffer buffer
+            (when privp (erc--unhide-prompt))
             ;; update the chat partner info.  Add to the list if private
             ;; message.  We will accumulate private identities indefinitely
             ;; at this point.
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 52fe106f2d..aab0b4c7ca 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -224,14 +224,24 @@ erc-send-whitespace-lines
   :group 'erc
   :type 'boolean)
 
-(defcustom erc-hide-prompt nil
-  "If non-nil, do not display the prompt for commands.
-
-\(A command is any input starting with a `/').
+(defcustom erc-prompt-hidden ">"
+  "Text to show in lieu of the prompt when hidden."
+  :package-version '(ERC . "5.4.1") ; FIXME increment on next ELPA release
+  :group 'erc-display
+  :type 'string)
 
-See also the variables `erc-prompt' and `erc-command-indicator'."
+(defcustom erc-hide-prompt t
+  "If non-nil, hide input prompt upon disconnecting.
+To unhide, type something in the input area.  Once revealed, a prompt
+remains unhidden until the next disconnection.  Channel prompts are
+unhidden upon rejoining.  Query prompts remain hidden until user input
+is detected or a new message arrives from the target."
+  :package-version '(ERC . "5.4.1") ; FIXME increment on next ELPA release
   :group 'erc-display
-  :type 'boolean)
+  :type '(choice (const :tag "Always hide prompt" t)
+                 (const :tag "Never hide prompt" nil)
+                 (const :tag "Only hide target prompt" 'target)
+                 (const :tag "Only hide server prompt" 'server)))
 
 ;; tunable GUI stuff
 
@@ -1996,7 +2006,7 @@ erc-open
         (buffer (erc-get-buffer-create server port channel))
         (old-buffer (current-buffer))
         old-point
-        continued-session)
+        (continued-session erc--server-reconnecting))
     (when connect (run-hook-with-args 'erc-before-connect server port nick))
     (erc-update-modules)
     (set-buffer buffer)
@@ -2014,7 +2024,7 @@ erc-open
     ;; (the buffer may have existed)
     (goto-char (point-max))
     (forward-line 0)
-    (when (get-text-property (point) 'erc-prompt)
+    (when (or continued-session (get-text-property (point) 'erc-prompt))
       (setq continued-session t)
       (set-marker erc-input-marker
                   (or (next-single-property-change (point) 'erc-prompt)
@@ -2074,7 +2084,8 @@ erc-open
       (goto-char (point-max))
       (insert "\n"))
     (if continued-session
-        (goto-char old-point)
+        (progn (goto-char old-point)
+               (erc--unhide-prompt))
       (set-marker erc-insert-marker (point))
       (erc-display-prompt)
       (goto-char (point-max)))
@@ -3745,9 +3756,9 @@ erc-cmd-RECONNECT
       (setq erc--server-reconnecting t)
       (setq erc-server-reconnect-count 0)
       (setq process (get-buffer-process (erc-server-buffer)))
-      (if process
-          (delete-process process)
-        (erc-server-reconnect))
+      (when process
+        (delete-process process))
+      (erc-server-reconnect)
       (with-suppressed-warnings ((obsolete erc-server-reconnecting))
         (setq erc-server-reconnecting nil))
       (setq erc--server-reconnecting nil)))
@@ -5665,27 +5676,6 @@ erc-send-input
             (erc-process-input-line (concat string "\n") t nil))
           t))))))
 
-;; (defun erc-display-command (line)
-;;   (when erc-insert-this
-;;     (let ((insert-position (point)))
-;;       (unless erc-hide-prompt
-;;         (erc-display-prompt nil nil (erc-command-indicator)
-;;                             (and (erc-command-indicator)
-;;                                  'erc-command-indicator-face)))
-;;       (let ((beg (point)))
-;;         (insert line)
-;;         (erc-put-text-property beg (point)
-;;                                'font-lock-face 'erc-command-indicator-face)
-;;         (insert "\n"))
-;;       (when (processp erc-server-process)
-;;         (set-marker (process-mark erc-server-process) (point)))
-;;       (set-marker erc-insert-marker (point))
-;;       (save-excursion
-;;         (save-restriction
-;;           (narrow-to-region insert-position (point))
-;;           (run-hooks 'erc-send-modify-hook)
-;;           (run-hooks 'erc-send-post-hook))))))
-
 (defun erc-display-msg (line)
   "Display LINE as a message of the user to the current target at point."
   (when erc-insert-this
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 373bed55af..c1d2196a38 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -128,6 +128,91 @@ erc-tests--set-fake-server-process
         (apply #'start-process (car args) (current-buffer) args))
   (set-process-query-on-exit-flag erc-server-process nil))
 
+(ert-deftest erc-hide-prompt ()
+  (let (erc-kill-channel-hook erc-kill-server-hook erc-kill-buffer-hook)
+
+    (with-current-buffer (get-buffer-create "ServNet")
+      (erc-tests--send-prep)
+      (goto-char erc-insert-marker)
+      (should (looking-at-p (regexp-quote erc-prompt)))
+      (erc-tests--set-fake-server-process "sleep" "1")
+      (set-process-sentinel erc-server-process #'ignore)
+      (setq erc-network 'ServNet)
+      (set-process-query-on-exit-flag erc-server-process nil))
+
+    (with-current-buffer (get-buffer-create "#chan")
+      (erc-tests--send-prep)
+      (goto-char erc-insert-marker)
+      (should (looking-at-p (regexp-quote erc-prompt)))
+      (setq erc-server-process (buffer-local-value 'erc-server-process
+                                                   (get-buffer "ServNet"))
+            erc-default-recipients '("#chan")))
+
+    (ert-info ("Value: t (default)")
+      (should (eq erc-hide-prompt t))
+      (with-current-buffer "ServNet"
+        (should (= (point) erc-insert-marker))
+        (erc--hide-prompt erc-server-process)
+        (should (string= ">" (get-text-property (point) 'display))))
+
+      (with-current-buffer "#chan"
+        (goto-char erc-insert-marker)
+        (should (string= ">" (get-text-property (point) 'display)))
+        (should (memq #'erc--unhide-prompt-on-self-insert pre-command-hook))
+        (goto-char erc-input-marker)
+        (ert-simulate-command '(self-insert-command 1 ?/))
+        (goto-char erc-insert-marker)
+        (should-not (get-text-property (point) 'display))
+        (should-not (memq #'erc--unhide-prompt-on-self-insert
+                          pre-command-hook)))
+
+      (with-current-buffer "ServNet"
+        (should (get-text-property erc-insert-marker 'display))
+        (should (memq #'erc--unhide-prompt-on-self-insert pre-command-hook))
+        (erc--unhide-prompt)
+        (should-not (memq #'erc--unhide-prompt-on-self-insert
+                          pre-command-hook))
+        (should-not (get-text-property erc-insert-marker 'display))))
+
+    (ert-info ("Value: server")
+      (setq erc-hide-prompt 'server)
+      (with-current-buffer "ServNet"
+        (erc--hide-prompt erc-server-process)
+        (should (string= ">" (get-text-property erc-insert-marker 'display))))
+
+      (with-current-buffer "#chan"
+        (should-not (get-text-property erc-insert-marker 'display)))
+
+      (with-current-buffer "ServNet"
+        (erc--unhide-prompt)
+        (should-not (get-text-property erc-insert-marker 'display))))
+
+    (ert-info ("Value: target")
+      (setq erc-hide-prompt 'target)
+      (with-current-buffer "ServNet"
+        (erc--hide-prompt erc-server-process)
+        (should-not (get-text-property erc-insert-marker 'display)))
+
+      (with-current-buffer "#chan"
+        (should (string= ">" (get-text-property erc-insert-marker 'display)))
+        (erc--unhide-prompt)
+        (should-not (get-text-property erc-insert-marker 'display))))
+
+    (ert-info ("Value: nil")
+      (setq erc-hide-prompt nil)
+      (with-current-buffer "ServNet"
+        (erc--hide-prompt erc-server-process)
+        (should-not (get-text-property erc-insert-marker 'display)))
+
+      (with-current-buffer "#chan"
+        (should-not (get-text-property erc-insert-marker 'display))
+        (erc--unhide-prompt) ; won't blow up when prompt already showing
+        (should-not (get-text-property erc-insert-marker 'display))))
+
+    (when noninteractive
+      (kill-buffer "#chan")
+      (kill-buffer "ServNet"))))
+
 (ert-deftest erc--switch-to-buffer ()
   (defvar erc-modified-channels-alist) ; lisp/erc/erc-track.el
 
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread
[parent not found: <87pmlqgfvg.fsf@neverwas.me>]
end of thread, other threads:[~2022-04-29 13:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-09 20:29 bug#54826: 29.0.50; Prevent duplicate prompts when reconnecting in ERC J.P.
     [not found] <87pmlqgfvg.fsf@neverwas.me>
2022-04-10 12:36 ` Lars Ingebrigtsen
     [not found] ` <8735ilqfn4.fsf@gnus.org>
2022-04-29 13:08   ` J.P.

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).