Re: bug#60936: 30.0.50; ERC >5.5: Add erc-fill style based on visual-line-mode
"J.P." <[email protected]>
| Newsgroups | gmane.emacs.erc.general |
|---|---|
| Message-ID | <[email protected]> |
Attached is some proposed follow-up work for this feature and also some
ancillary changes that I might spin off into a new bug.
The first patch adds a couple helpers that encapsulate some of the
tedium involved in warning about missing dependencies and persisting
state between sessions. The second is more trivial and provides a
feature that adds spacing between certain types of messages. This only
works on graphical displays and pairs well with the variable
`line-spacing' and the new `fill-wrap' module. The third patch is the
simplest but also the most consequential. It's meant to illustrate one
way of tackling the problem of late module activation due to missing
dependencies, which (to my knowledge) mainly concerns two use cases:
1. A local module depends on a global one that hooks on
`erc-mode-hook', meaning the dependent has no way of activating the
dependency properly just by calling its minor-mode activation
toggle alone (because `erc-mode-hook' won't run again).
2. A user manually invokes a global module's minor mode toggle (in
code or interactively) in an existing, established ERC session.
Again we face the problem of whatever portion of the module's setup
lives in `erc-mode-hook' not being invited to run.
Both would be non-issues if we suddenly decided to adopt a policy of
canceling activation when encountering unmet dependencies. But barring
that, what I'm proposing is finding some way to fully activate a
dependency by invoking its mode activation toggle alone, both in the
vicinity of `erc-connect-pre-hook' (1) and elsewhere, away from any
session-init-related happenings (2).
As things stand, dependents can't activate dependencies that are only
designed to work in subsequent sessions because they may render all
current sessions instantly unusable. These "foundational" dependencies,
like 'stamp' and 'button', typically add members to insert-related
hooks, like `erc-insert-pre-hook' but also add setup-related business to
`erc-mode-hook'. Predictably, the insert variety run as soon as the next
message arrives, even though they likely depend on setup performed by
the other type, which only run when ERC creates new buffers.
The second case (2) is rare by comparison because modules designed to be
activated in established sessions have a more dynamic, transitory focus,
such as turning on spell checking or managing a side window, and are
thus already outfitted with the necessary resilience. By contrast,
foundational modules tend to apply destructive operations to buffers and
thus can't be toggled without upsetting the user experience
significantly (which is why, I'm guessing, their authors didn't even
bother taking similar precautions since there's never been a sane use
case). However, there's recently been cause to create a global module of
type 2 that depends on the flimsier, more foundational type 1, hence my
mentioning this second type.
With all that in mind, here are two possible approaches to tackling
this problem (though others undoubtedly exist [1]):
a. Simple context flags in `erc-open'
One or more variables to be bound non-nil around various
module-setup contexts, such as when running deferred
`erc-mode-hook' members or during the initial call to
`erc--update-modules'
Pros:
- modules can opt in to contexts on a granular level
- implementations are straightforward and appear directly in
module setup, guarded by these flag variables
Cons:
- slightly verbose boilerplate that's tricky to provide a
convenience helper for given the variations involved with
meeting the idempotency and resilience requirements of "run
wherever, whenever"
- dependents have no way of detecting whether a dependency is
"context aware" and thus safe to run (unless documented)
because these flags are purely optional
b. Convenience helper + symbol property
Modules opt in by marking functions added to `erc-mode-hook' with a
symbol property, like so:
(put 'erc-foo-setup 'erc-mode-hook 'erc-foo-mode)
And when it comes time for dependents to activate such a
dependency, they can avail themselves of a helper, which calls any
marked `erc-mode-hook' member of interest in all ERC buffers.
Pros:
- dependent modules have a trusted means of detecting whether a
dependency's toggle is safely runnable anywhere
Cons:
- dependent modules must use the helper to activate dependencies,
meaning this approach does not obviate the need for special
code in those modules supporting activation during established
sessions (type 2, above), however it does take care of _their_
dependencies, which is the main point
- members added to `erc-mode-hook' by a participating dependency
must still be made resilient manually
For no particular reason, I've gone with (a) in the attached patch set,
but am obviously amenable to suggestions.
Thanks.
[1] I'm pretty sure it's impractical to try solving this problem by
going to extremes, such as by rewriting every `erc-mode-hook' member
to be ultra cautious or, on the flip side, opting for something
totally magical. By the latter, I mostly mean trying to cleverly
modify `define-erc-module' to statically detect occurrences of forms
like (add-hook 'erc-mode-hook #'foo) and arrange for them to run as
in (a), above. This is likely not worth attempting because some
modules subscribe to that hook indirectly. There's also the problem
of making modules aware of the new paradigm, since they may well
have code that's unfit to run in various contexts.
0001-5.6-Add-helper-for-restoring-local-session-vars-in-E.patch
(text/x-patch, 14.5 KB)
From 1b064c410393843e7d07f2b3ac70d42c729872fb Mon Sep 17 00:00:00 2001 From: "F. Jason Park" <[email protected]> Date: Sun, 7 May 2023 19:43:57 -0700 Subject: [PATCH 1/3] [5.6] Add helper for restoring local session vars in ERC * lisp/erc/erc-goodies.el (erc--keep-place-indicator-setup): Use macro `erc--restore-initialize-priors'. (erc-keep-place-indicator-mode, erc-keep-place-indicator-enable, erc-keep-place-indicator-disable): Use convenience function to show missing-dependency notice. * lisp/erc/erc-sasl.el (erc-sasl-auth-source-password-as-host): Merge redundant `when' forms. (erc-sasl--init): Remove unused function. (erc-sasl-mode, erc-sasl-enable, erc-sasl-disable): Use helper `erc--restore-initialize-priors' to restore `erc-sasl--options' * lisp/erc/erc.el (erc--restore-initialize-priors): New macro. (erc--warn-once-before-connect): Add helper to display messages, ideally just after module setup. * test/lisp/erc/erc-tests.el (erc--restore-initialize-priors): New test. Also see test/lisp/erc/erc-scenarios-base-local-modules.el for more realistic exercising of this functionality. * test/lisp/erc/erc-goodies-tests.el (erc-controls-highlight--examples, erc-controls-highlight--inverse, erc-controls-highlight--motd, erc-keep-place-indicator-mode): Remove feature check. For the latter, also start fake process and shadow `erc-connect-pre-hook'. (Bug#60936) --- lisp/erc/erc-goodies.el | 24 ++++++---------- lisp/erc/erc-sasl.el | 38 +++++++++++-------------- lisp/erc/erc.el | 45 ++++++++++++++++++++++++++++++ test/lisp/erc/erc-goodies-tests.el | 19 ++++--------- test/lisp/erc/erc-tests.el | 15 ++++++++++ 5 files changed, 90 insertions(+), 51 deletions(-) diff --git a/lisp/erc/erc-goodies.el b/lisp/erc/erc-goodies.el index cc60ba0018b..4558ff7c076 100644 --- a/lisp/erc/erc-goodies.el +++ b/lisp/erc/erc-goodies.el @@ -154,21 +154,21 @@ erc-keep-place-indicator-style `face', ERC adds the face `erc-keep-place-indicator-line' to the appropriate line. A value of t does both." :group 'erc - :package-version '(ERC . "5.6") + :package-version '(ERC . "5.6") ; FIXME sync on release :type '(choice (const t) (const server) (const target))) (defcustom erc-keep-place-indicator-buffer-type t "ERC buffer type in which to display `keep-place-indicator'. A value of t means \"all\" ERC buffers." :group 'erc - :package-version '(ERC . "5.6") + :package-version '(ERC . "5.6") ; FIXME sync on release :type '(choice (const t) (const server) (const target))) (defcustom erc-keep-place-indicator-follow nil "Whether to sync visual kept place to window's top when reading. For use with `erc-keep-place-indicator-mode'." :group 'erc - :package-version '(ERC . "5.6") + :package-version '(ERC . "5.6") ; FIXME sync on release :type 'boolean) (defface erc-keep-place-indicator-line @@ -209,11 +209,8 @@ erc--keep-place-indicator-on-window-configuration-change (defun erc--keep-place-indicator-setup () "Initialize buffer for maintaining `erc--keep-place-indicator-overlay'." (require 'fringe) - (setq erc--keep-place-indicator-overlay - (if-let* ((vars (or erc--server-reconnecting erc--target-priors)) - ((alist-get 'erc-keep-place-indicator-mode vars))) - (alist-get 'erc--keep-place-indicator-overlay vars) - (make-overlay 0 0))) + (erc--restore-initialize-priors erc-keep-place-indicator-mode + erc--keep-place-indicator-overlay (make-overlay 0 0)) (add-hook 'window-configuration-change-hook #'erc--keep-place-indicator-on-window-configuration-change nil t) (when-let* (((memq erc-keep-place-indicator-style '(t arrow))) @@ -232,13 +229,10 @@ keep-place-indicator "`keep-place' with a fringe arrow and/or highlighted face." ((unless erc-keep-place-mode (unless (memq 'keep-place erc-modules) - ;; FIXME use `erc-button--display-error-notice-with-keys' - ;; to display this message when bug#60933 is ready. - (erc-display-error-notice - nil (concat - "Local module `keep-place-indicator' needs module `keep-place'." - " Enabling now. This will affect \C-]all\C-] ERC sessions." - " Add `keep-place' to `erc-modules' to silence this message."))) + (erc--warn-once-before-connect 'erc-keep-place-mode + "Local module `keep-place-indicator' needs module `keep-place'." + " Enabling now. This will affect \C-]all\C-] ERC sessions." + " Add `keep-place' to `erc-modules' to silence this message.")) (erc-keep-place-mode +1)) (if (pcase erc-keep-place-indicator-buffer-type ('target erc--target) diff --git a/lisp/erc/erc-sasl.el b/lisp/erc/erc-sasl.el index bfe17285a68..c6922b1b26b 100644 --- a/lisp/erc/erc-sasl.el +++ b/lisp/erc/erc-sasl.el @@ -137,12 +137,12 @@ erc-sasl-auth-source-password-as-host `erc-session-password' instead. Otherwise, just defer to `erc-auth-source-search' to pick a suitable `:host'. Expect PLIST to contain keyword params known to `auth-source-search'." - (when erc-sasl-password - (when-let ((host (if (eq :password erc-sasl-password) - (and (not (functionp erc-session-password)) - erc-session-password) - erc-sasl-password))) - (setq plist `(,@plist :host ,(format "%s" host))))) + (when-let* ((erc-sasl-password) + (host (if (eq :password erc-sasl-password) + (and (not (functionp erc-session-password)) + erc-session-password) + erc-sasl-password))) + (setq plist `(,@plist :host ,(format "%s" host)))) (apply #'erc-auth-source-search plist)) (defun erc-sasl--read-password (prompt) @@ -297,21 +297,6 @@ erc-sasl--create-client (sasl-client-set-property client 'ecdsa-keyfile keyfile) client))))) -;; This stands alone because it's also used by bug#49860. -(defun erc-sasl--init () - (setq erc-sasl--state (make-erc-sasl--state)) - ;; If the previous attempt failed during registration, this may be - ;; non-nil and contain erroneous values, but how can we detect that? - ;; What if the server dropped the connection for some other reason? - (setq erc-sasl--options - (or (and erc--server-reconnecting - (alist-get 'erc-sasl--options erc--server-reconnecting)) - `((user . ,erc-sasl-user) - (password . ,erc-sasl-password) - (mechanism . ,erc-sasl-mechanism) - (authfn . ,erc-sasl-auth-source-function) - (authzid . ,erc-sasl-authzid))))) - (defun erc-sasl--mechanism-offered-p (offered) "Return non-nil when OFFERED appears among a list of mechanisms." (string-match-p (rx-to-string @@ -334,7 +319,16 @@ sasl This doesn't solicit or validate a suite of supported mechanisms." ;; See bug#49860 for a CAP 3.2-aware WIP implementation. ((unless erc--target - (erc-sasl--init) + (setq erc-sasl--state (make-erc-sasl--state)) + ;; If the previous attempt failed during registration, this may be + ;; non-nil and contain erroneous values, but how can we detect that? + ;; What if the server dropped the connection for some other reason? + (erc--restore-initialize-priors erc-sasl-mode + erc-sasl--options `((user . ,erc-sasl-user) + (password . ,erc-sasl-password) + (mechanism . ,erc-sasl-mechanism) + (authfn . ,erc-sasl-auth-source-function) + (authzid . ,erc-sasl-authzid))) (let* ((mech (alist-get 'mechanism erc-sasl--options)) (client (erc-sasl--create-client mech))) (unless client diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el index dbf413bac74..249b9963cea 100644 --- a/lisp/erc/erc.el +++ b/lisp/erc/erc.el @@ -1363,6 +1363,20 @@ erc--target-priors Bound to local variables from an existing (logical) session's buffer during local-module setup and `erc-mode-hook' activation.") +(defmacro erc--restore-initialize-priors (mode &rest vars) + "Restore local VARS for MODE from a previous session." + (declare (indent 1)) + (let ((existing (make-symbol "existing")) + ;; + restore initialize) + (while-let ((k (pop vars)) (v (pop vars))) + (push `(,k (alist-get ',k ,existing)) restore) + (push `(,k ,v) initialize)) + `(if-let* ((,existing (or erc--server-reconnecting erc--target-priors)) + ((alist-get ',mode ,existing))) + (setq ,@(mapcan #'identity (nreverse restore))) + (setq ,@(mapcan #'identity (nreverse initialize)))))) + (defun erc--target-from-string (string) "Construct an `erc--target' variant from STRING." (funcall (if (erc-channel-p string) @@ -1412,6 +1426,37 @@ erc-once-with-server-event (add-hook hook fun nil t) fun)) +(defun erc--warn-once-before-connect (mode-var &rest args) + "Display an \"error notice\" once. +Expect ARGS to be `erc-button--display-error-notice-with-keys' +compatible parameters, except without any leading buffers or +processes. If we're in an ERC buffer with a network process when +called, print the notice immediately. Otherwise, if we're in a +server buffer, arrange to do so after local modules have been set +up and mode hooks have run. Otherwise, if MODE-VAR is a global +module, try again at most once the next time `erc-mode-hook' +runs." + (declare (indent 1)) + (cl-assert (stringp (car args))) + (if (derived-mode-p 'erc-mode) + (unless (or (erc-with-server-buffer ; needs `erc-server-process' + (apply #'erc-button--display-error-notice-with-keys + (current-buffer) args) + t) + erc--target) ; unlikely + (let (hook) + (setq hook + (lambda (_) + (remove-hook 'erc-connect-pre-hook hook t) + (apply #'erc-button--display-error-notice-with-keys args))) + (add-hook 'erc-connect-pre-hook hook nil t))) + (when (custom-variable-p mode-var) + (let (hook) + (setq hook (lambda () + (remove-hook 'erc-mode-hook hook) + (apply #'erc--warn-once-before-connect 'erc-fake args))) + (add-hook 'erc-mode-hook hook))))) + (defun erc-server-buffer () "Return the server buffer for the current buffer's process. The buffer-local variable `erc-server-process' is used to find diff --git a/test/lisp/erc/erc-goodies-tests.el b/test/lisp/erc/erc-goodies-tests.el index a1f53c5bf88..bd40df4b680 100644 --- a/test/lisp/erc/erc-goodies-tests.el +++ b/test/lisp/erc/erc-goodies-tests.el @@ -21,7 +21,6 @@ ;;; Code: (require 'ert-x) (require 'erc-goodies) -(declare-function erc--initialize-markers "erc" (old-point continued) t) (defun erc-goodies-tests--assert-face (beg end-str present &optional absent) (setq beg (+ beg (point-min))) @@ -44,9 +43,6 @@ erc-goodies-tests--assert-face ;; https://modern.ircdocs.horse/formatting.html (ert-deftest erc-controls-highlight--examples () - ;; FIXME remove after adding - (unless (fboundp 'erc--initialize-markers) - (ert-skip "Missing required function")) (should (eq t erc-interpret-controls-p)) (let ((erc-insert-modify-hook '(erc-controls-highlight)) erc-kill-channel-hook erc-kill-server-hook erc-kill-buffer-hook) @@ -130,9 +126,6 @@ erc-controls-highlight--examples ;; in a high-contrast face. (ert-deftest erc-controls-highlight--inverse () - ;; FIXME remove after adding - (unless (fboundp 'erc--initialize-markers) - (ert-skip "Missing required function")) (should (eq t erc-interpret-controls-p)) (let ((erc-insert-modify-hook '(erc-controls-highlight)) erc-kill-channel-hook erc-kill-server-hook erc-kill-buffer-hook) @@ -212,9 +205,6 @@ erc-goodies-tests--motd (":- "))) (ert-deftest erc-controls-highlight--motd () - ;; FIXME remove after adding - (unless (fboundp 'erc--initialize-markers) - (ert-skip "Missing required function")) (should (eq t erc-interpret-controls-p)) (let ((erc-insert-modify-hook '(erc-controls-highlight)) erc-kill-channel-hook erc-kill-server-hook erc-kill-buffer-hook) @@ -256,12 +246,12 @@ erc-controls-highlight--motd ;; needed. (ert-deftest erc-keep-place-indicator-mode () - ;; FIXME remove after adding - (unless (fboundp 'erc--initialize-markers) - (ert-skip "Missing required function")) (with-current-buffer (get-buffer-create "*erc-keep-place-indicator-mode*") (erc-mode) (erc--initialize-markers (point) nil) + (setq erc-server-process + (start-process "sleep" (current-buffer) "sleep" "1")) + (set-process-query-on-exit-flag erc-server-process nil) (let ((assert-off (lambda () (should-not erc-keep-place-indicator-mode) @@ -275,6 +265,7 @@ erc-keep-place-indicator-mode (should erc-keep-place-mode))) ;; erc-insert-pre-hook + erc-connect-pre-hook erc-modules) (funcall assert-off) @@ -284,7 +275,7 @@ erc-keep-place-indicator-mode (erc-keep-place-indicator-mode +1) (funcall assert-on) (goto-char (point-min)) - (should (search-forward "Enabling" nil t)) + (should (search-forward "Enabling now" nil t)) (should (memq 'keep-place erc-modules))) (erc-keep-place-indicator-mode -1) diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el index be5a566a268..b624186d88d 100644 --- a/test/lisp/erc/erc-tests.el +++ b/test/lisp/erc/erc-tests.el @@ -868,6 +868,21 @@ erc--valid-local-channel-p (should-not (erc--valid-local-channel-p "#chan")) (should (erc--valid-local-channel-p "&local"))))) +(ert-deftest erc--restore-initialize-priors () + ;; This `pcase' expands to 100+k. Guess we could do something like + ;; (and `(,_ ((,e . ,_) . ,_) . ,_) v) first and then return a + ;; (equal `(if-let* ((,e ...)...)...) v) to cut it down to < 1k. + (should (pcase (macroexpand-1 '(erc--restore-initialize-priors erc-my-mode + foo (ignore 1 2 3) + bar #'spam)) + (`(if-let* ((,e (or erc--server-reconnecting erc--target-priors)) + ((alist-get 'erc-my-mode ,e))) + (setq foo (alist-get 'foo ,e) + bar (alist-get 'bar ,e)) + (setq foo (ignore 1 2 3) + bar #'spam)) + t)))) + (ert-deftest erc--target-from-string () (should (equal (erc--target-from-string "#chan") #s(erc--target-channel "#chan" \#chan))) -- 2.40.0
0002-5.6-Optionally-add-spacing-between-ERC-messages.patch
(text/x-patch, 17.8 KB)
From 18ca3e42ba2740b59e3172f344f8b5c22c66014c Mon Sep 17 00:00:00 2001 From: "F. Jason Park" <[email protected]> Date: Sun, 7 May 2023 07:28:56 -0700 Subject: [PATCH 2/3] [5.6] Optionally add spacing between ERC messages * etc/ERC-NEWS: Mention options `erc-fill-line-spacing' and `erc-fill-spaced-commands'. * lisp/erc/erc-fill.el (erc-fill-line-spacing, erc-fill-spaced-commands): Add options to allow for extra spacing between messages. (erc-fill--function): Internal var for local modules. (erc-fill): Add extra line-spacing on certain types of messages. Prefer `erc-fill--function', when set, over `erc-fill-function'. (erc-fill--make-module-dependency-msg, erc-fill--wrap-ensure-dependencies): Rename and make more useful. (erc-fill-wrap-mode, erc-fill-wrap-enable, erc-fill-wrap-disable): Refactor. (erc-fill--wrap-fix): Remove unused function. (erc-fill-wrap-nudge): Remove reference to nonexistent function in doc string. * test/lisp/erc/erc-fill-tests.el: (erc-fill-tests--graphic-dir): New variable. (erc-fill-tests--compare): Look in `erc-fill-tests--graphic-dir' for graphical snapshots. (erc-fill-line-spacing): New test. * test/lisp/erc/resources/fill/snapshots/spacing-01-mono.eld: New file. (Bug#60936) --- etc/ERC-NEWS | 7 + lisp/erc/erc-fill.el | 166 +++++++++--------- test/lisp/erc/erc-fill-tests.el | 24 ++- .../fill/snapshots/spacing-01-mono.eld | 1 + 4 files changed, 113 insertions(+), 85 deletions(-) create mode 100644 test/lisp/erc/resources/fill/snapshots/spacing-01-mono.eld diff --git a/etc/ERC-NEWS b/etc/ERC-NEWS index f2a8eb72b95..5740548facb 100644 --- a/etc/ERC-NEWS +++ b/etc/ERC-NEWS @@ -90,6 +90,13 @@ from the same connection. This customization depends on the option 'frame'. If you find the name 'displayed' unhelpful, please suggest an alternative by writing to the mailing list. +** Module 'fill' can add a bit of space between messages. +On graphical displays, it's now possible to add some breathing room +around certain messages to make their boundaries more distinguishable, +especially when repeated speaker tags are omitted, such as when using +the new option 'erc-fill-wrap-merge'. See 'erc-fill-line-spacing' to +get started. + ** Some keybindings are now set by modules rather than their libraries. To put it another way, simply loading a built-in module's library no longer modifies 'erc-mode-map'. Instead, modifications occur during diff --git a/lisp/erc/erc-fill.el b/lisp/erc/erc-fill.el index bf995a5a5e6..a34e0aa6eb4 100644 --- a/lisp/erc/erc-fill.el +++ b/lisp/erc/erc-fill.el @@ -116,12 +116,30 @@ erc-fill-column "The column at which a filled paragraph is broken." :type 'integer) +(defcustom erc-fill-line-spacing nil + "Extra space between messages on graphical displays. +This may need adjusting depending on how your faces are +configured. Its value should be larger than that of the variable +`line-spacing', if set. If unsure, try 0.5." + :package-version '(ERC . "5.6") ; FIXME sync on release + :type '(choice (const nil) number)) + +(defcustom erc-fill-spaced-commands '(PRIVMSG NOTICE) + "Types of mesages to add space between on graphical displays. +Only considered when `erc-fill-line-spacing' is non-nil." + :package-version '(ERC . "5.6") ; FIXME sync on release + :type '(set integer symbol)) + +(defvar-local erc-fill--function nil + "Internal copy of `erc-fill-function'. +Takes precedence over the latter when non-nil.") + ;;;###autoload (defun erc-fill () "Fill a region using the function referenced in `erc-fill-function'. You can put this on `erc-insert-modify-hook' and/or `erc-send-modify-hook'." (unless (erc-string-invisible-p (buffer-substring (point-min) (point-max))) - (when erc-fill-function + (when (or erc-fill--function erc-fill-function) ;; skip initial empty lines (goto-char (point-min)) (save-match-data @@ -130,7 +148,19 @@ erc-fill (unless (eobp) (save-restriction (narrow-to-region (point) (point-max)) - (funcall erc-fill-function)))))) + (funcall (or erc-fill--function erc-fill-function)) + (when-let* ((erc-fill-line-spacing) + (p (point-min))) + (widen) + (when (or (and-let* ((cmd (get-text-property p 'erc-command))) + (memq cmd erc-fill-spaced-commands)) + (and-let* ((cmd (save-excursion + (forward-line -1) + (get-text-property (point) + 'erc-command)))) + (memq cmd erc-fill-spaced-commands))) + (put-text-property (1- p) p + 'line-spacing erc-fill-line-spacing)))))))) (defun erc-fill-static () "Fills a text such that messages start at column `erc-fill-static-center'." @@ -264,71 +294,66 @@ erc-match-mode (defvar erc-button-mode) (defvar erc-match--hide-fools-offset-bounds) -(defun erc-fill--make-module-dependency-msg (module) - (concat "Enabling default global module `" module "' needed by local" - " module `fill-wrap'. This will impact \C-]all\C-] ERC" - " sessions. Add `" module "' to `erc-modules' to avoid this" - " warning. See Info:\"(erc) Modules\" for more.")) +;; This stands alone in hopes some internal framework will eventually +;; emerge to cover this common boilerplate. The same goes for +;; restoring `erc--server-reconnecting' and `erc--target-priors'. +(defun erc-fill--wrap-ensure-dependencies () + (let (missing-deps) + (unless erc-fill-mode + (unless (memq 'fill erc-modules) (push 'fill missing-deps)) + (erc-fill-mode +1)) + (when erc-fill-wrap-merge + (require 'erc-button) + (unless erc-button-mode + (unless (memq 'button erc-modules) (push 'button missing-deps)) + (erc-button-mode +1)) + (require 'erc-stamp) + (unless erc-stamp-mode + (unless (memq 'stamp erc-modules) (push 'stamp missing-deps)) + (erc-stamp-mode +1))) + (when missing-deps + (erc--warn-once-before-connect 'erc-fill-wrap-mode + "Enabling missing global modules %s needed by local" + " module `fill-wrap'. This will impact \C-]all\C-] ERC" + " sessions. Add them to `erc-modules' to avoid this" + " warning. See Info:\"(erc) Modules\" for more." + (mapcar (lambda (s) (format "`%s'" s)) missing-deps))))) ;;;###autoload(put 'fill-wrap 'erc--feature 'erc-fill) (define-erc-module fill-wrap nil "Fill style leveraging `visual-line-mode'. -This module displays nickname labels for speakers as overhanging -leftward (and thus right-aligned) to a common offset, as -determined by the option `erc-fill-static-center'. It depends on -the `fill' and `button' modules and assumes the option +This local module displays nicks overhanging leftward to a common +offset, as determined by the option `erc-fill-static-center'. It +depends on the `fill' and `button' modules and assumes the option `erc-insert-timestamp-function' is `erc-insert-timestamp-right' -or `erc-insert-timestamp-left-and-right' (recommended) so that it +or the default `erc-insert-timestamp-left-and-right', so that it can display right-hand stamps in the right margin. A value of -`erc-insert-timestamp-left' is unsupported. This local module -depends on the global `fill' module. To use it, either include -`fill-wrap' in `erc-modules' or set `erc-fill-function' to -`erc-fill-wrap' (recommended). You can also manually invoke one -of the minor-mode toggles as usual." - ((let (msg) - (unless erc-fill-mode - (unless (memq 'fill erc-modules) - (setq msg - ;; FIXME use `erc-button--display-error-notice-with-keys' - ;; when bug#60933 is ready. - (erc-fill--make-module-dependency-msg "fill"))) - (erc-fill-mode +1)) - (when erc-fill-wrap-merge - (require 'erc-button) - (unless erc-button-mode - (unless (memq 'button erc-modules) - (setq msg (concat msg (and msg " ") - (erc-fill--make-module-dependency-msg "button")))) - (erc-with-server-buffer - (erc-button-mode +1))) - (add-hook 'erc-button--prev-next-predicate-functions - #'erc-fill--wrap-merged-button-p nil t)) - ;; Set local value of user option (can we avoid this somehow?) - (unless (eq erc-fill-function #'erc-fill-wrap) - (setq-local erc-fill-function #'erc-fill-wrap)) - (when-let* ((vars (or erc--server-reconnecting erc--target-priors)) - ((alist-get 'erc-fill-wrap-mode vars))) - (setq erc-fill--wrap-visual-keys (alist-get 'erc-fill--wrap-visual-keys - vars) - erc-fill--wrap-value (alist-get 'erc-fill--wrap-value vars))) - (add-function :filter-args (local 'erc-stamp--insert-date-function) - #'erc-fill--wrap-stamp-insert-prefixed-date) - (when (or erc-stamp-mode (memq 'stamp erc-modules)) - (erc-stamp--display-margin-mode +1)) - (when (or (bound-and-true-p erc-match-mode) (memq 'match erc-modules)) - (require 'erc-match) - (setq erc-match--hide-fools-offset-bounds t)) - (setq erc-fill--wrap-value - (or erc-fill--wrap-value erc-fill-static-center)) - (visual-line-mode +1) - (unless (local-variable-p 'erc-fill--wrap-visual-keys) - (setq erc-fill--wrap-visual-keys erc-fill-wrap-visual-keys)) - (when msg - (erc-display-error-notice nil msg)))) +`erc-insert-timestamp-left' is unsupported. To use it, either +include `fill-wrap' in `erc-modules' or set `erc-fill-function' +to `erc-fill-wrap' (recommended). You can also manually invoke +one of the minor-mode toggles if really necessary." + ((erc-fill--wrap-ensure-dependencies) + ;; Restore or initialize local state variables. + (erc--restore-initialize-priors erc-fill-wrap-mode + erc-fill--wrap-visual-keys erc-fill-wrap-visual-keys + erc-fill--wrap-value erc-fill-static-center) + (setq erc-fill--function #'erc-fill-wrap) + ;; Internal integrations. + (add-function :filter-args (local 'erc-stamp--insert-date-function) + #'erc-fill--wrap-stamp-insert-prefixed-date) + (when (or erc-stamp-mode (memq 'stamp erc-modules)) + (erc-stamp--display-margin-mode +1)) + (when (or (bound-and-true-p erc-match-mode) (memq 'match erc-modules)) + (require 'erc-match) + (setq erc-match--hide-fools-offset-bounds t)) + (when erc-fill-wrap-merge + (add-hook 'erc-button--prev-next-predicate-functions + #'erc-fill--wrap-merged-button-p nil t)) + (visual-line-mode +1)) ((when erc-stamp--display-margin-mode (erc-stamp--display-margin-mode -1)) (kill-local-variable 'erc-fill--wrap-value) - (kill-local-variable 'erc-fill-function) + (kill-local-variable 'erc-fill--function) (kill-local-variable 'erc-fill--wrap-visual-keys) (remove-hook 'erc-button--prev-next-predicate-functions #'erc-fill--wrap-merged-button-p t) @@ -422,28 +447,6 @@ erc-fill-wrap (defun erc-fill--wrap-merged-button-p (point) (equal "" (get-text-property point 'display))) -;; This is an experimental helper for third-party modules. You could, -;; for example, use this to automatically resize the prefix to a -;; fraction of the window's width on some event change. Another use -;; case would be to fix lines affected by toggling a display-oriented -;; mode, like `display-line-numbers-mode'. - -(defun erc-fill--wrap-fix (&optional value) - "Re-wrap from `point-min' to `point-max'. -That is, recalculate the width of all accessible lines and reset -local prefix VALUE when non-nil." - (save-excursion - (when value - (setq erc-fill--wrap-value value)) - (let ((inhibit-field-text-motion t) - (inhibit-read-only t)) - (goto-char (point-min)) - (while (and (zerop (forward-line)) - (< (point) (min (point-max) erc-insert-marker))) - (save-restriction - (narrow-to-region (line-beginning-position) (line-end-position)) - (erc-fill-wrap)))))) - (defun erc-fill--wrap-nudge (arg) (when (zerop arg) (setq arg (- erc-fill-static-center erc-fill--wrap-value))) @@ -463,8 +466,7 @@ erc-fill-wrap-nudge \\`)' Reset the right margin to the default Note that misalignment may occur when messages contain -decorations applied by third-party modules. See -`erc-fill--wrap-fix' for a temporary workaround." +decorations applied by third-party modules." (interactive "p") (unless erc-fill--wrap-value (cl-assert (not erc-fill-wrap-mode)) diff --git a/test/lisp/erc/erc-fill-tests.el b/test/lisp/erc/erc-fill-tests.el index 170436ffbaa..fc33d0b9103 100644 --- a/test/lisp/erc/erc-fill-tests.el +++ b/test/lisp/erc/erc-fill-tests.el @@ -120,10 +120,14 @@ erc-fill-tests--wrap-check-prefixes ;; Obviously, only run one test at a time. (defvar erc-fill-tests--save-p nil) +;; On graphical displays, echo .graphic >> .git/info/exclude +(defvar erc-fill-tests--graphic-dir "fill/snapshots/.graphic") + (defun erc-fill-tests--compare (name) - (when (display-graphic-p) - (setq name (concat name "-graphic"))) - (let* ((dir (expand-file-name "fill/snapshots/" (ert-resource-directory))) + (let* ((dir (expand-file-name (if (display-graphic-p) + erc-fill-tests--graphic-dir + "fill/snapshots/") + (ert-resource-directory))) (expect-file (file-name-with-extension (expand-file-name name dir) "eld")) (erc--own-property-names @@ -232,6 +236,20 @@ erc-fill-wrap--merge "<bob> " "<alice> " "<alice> " "<bob> " "<bob> " "<Dummy> " "<Dummy> ") (erc-fill-tests--compare "merge-02-right"))))) +(ert-deftest erc-fill-line-spacing () + :tags '(:unstable) + (unless (>= emacs-major-version 29) + (ert-skip "Emacs version too low, missing `buffer-text-pixel-size'")) + + (let ((erc-fill-line-spacing 0.5)) + (erc-fill-tests--wrap-populate + (lambda () + (erc-fill-tests--insert-privmsg "bob" "This buffer is for text.") + (erc-display-message nil 'notice (current-buffer) "one two three") + (erc-display-message nil 'notice (current-buffer) "four five six") + (erc-fill-tests--insert-privmsg "bob" "Somebody stop me") + (erc-fill-tests--compare "spacing-01-mono"))))) + (ert-deftest erc-fill-wrap-visual-keys--body () :tags '(:unstable) (erc-fill-tests--wrap-populate diff --git a/test/lisp/erc/resources/fill/snapshots/spacing-01-mono.eld b/test/lisp/erc/resources/fill/snapshots/spacing-01-mono.eld new file mode 100644 index 00000000000..45c3883b023 --- /dev/null +++ b/test/lisp/erc/resources/fill/snapshots/spacing-01-mono.eld @@ -0,0 +1 @@ +#("\n\n\n[Thu Jan 1 1970]\n*** This server is in debug mode and is logging all user I/O. If you do not wish for everything you send to be readable by the server owner(s), please disconnect.[00:00]\n<alice> bob: come, you are a tedious fool: to the purpose. What was done to Elbow's wife, that he hath cause to complain of? Come me to what was done to her.\n<bob> alice: Either your unparagoned mistress is dead, or she's outprized by a trifle.\n<bob> This buffer is for text.\n*** one two three\n*** four five six\n<bob> Somebody stop me\n" 2 21 (erc-timestamp 0 line-prefix (space :width (- 27 18)) field erc-timestamp) 21 183 (erc-timestamp 0 wrap-prefix #2=(space :width 27) line-prefix #3=(space :width (- 27 (4)))) 183 190 (erc-timestamp 0 field erc-timestamp wrap-prefix #2# line-prefix #3# display #1=((margin right-margin) #("[00:00]" 0 7 (display #1# isearch-open-invisible timestamp invisible timestamp font-lock-face erc-timestamp-face)))) 190 191 (line-spacing 0.5 wrap-prefix #2# line-prefix #3#) 191 192 (erc-timestamp 0 wrap-prefix #2# line-prefix #4=(space :width (- 27 (8))) erc-command PRIVMSG) 192 197 (erc-timestamp 0 wrap-prefix #2# line-prefix #4# erc-command PRIVMSG) 197 199 (erc-timestamp 0 wrap-prefix #2# line-prefix #4# erc-command PRIVMSG) 199 202 (erc-timestamp 0 wrap-prefix #2# line-prefix #4# erc-command PRIVMSG) 202 315 (erc-timestamp 0 wrap-prefix #2# line-prefix #4# erc-command PRIVMSG) 315 316 (erc-timestamp 0 erc-command PRIVMSG) 316 348 (erc-timestamp 0 wrap-prefix #2# line-prefix #4# erc-command PRIVMSG) 348 349 (line-spacing 0.5 wrap-prefix #2# line-prefix #4#) 349 350 (erc-timestamp 0 wrap-prefix #2# line-prefix #5=(space :width (- 27 (6))) erc-command PRIVMSG) 350 353 (erc-timestamp 0 wrap-prefix #2# line-prefix #5# erc-command PRIVMSG) 353 355 (erc-timestamp 0 wrap-prefix #2# line-prefix #5# erc-command PRIVMSG) 355 360 (erc-timestamp 0 wrap-prefix #2# line-prefix #5# erc-command PRIVMSG) 360 435 (erc-timestamp 0 wrap-prefix #2# line-prefix #5# erc-command PRIVMSG) 435 436 (line-spacing 0.5 wrap-prefix #2# line-prefix #5#) 436 437 (erc-timestamp 0 wrap-prefix #2# line-prefix #6=(space :width (- 27 0)) display #7="" erc-command PRIVMSG) 437 440 (erc-timestamp 0 wrap-prefix #2# line-prefix #6# display #7# erc-command PRIVMSG) 440 442 (erc-timestamp 0 wrap-prefix #2# line-prefix #6# display #7# erc-command PRIVMSG) 442 466 (erc-timestamp 0 wrap-prefix #2# line-prefix #6# erc-command PRIVMSG) 466 467 (line-spacing 0.5 wrap-prefix #2# line-prefix #6#) 467 484 (erc-timestamp 0 wrap-prefix #2# line-prefix #8=(space :width (- 27 (4)))) 484 485 (wrap-prefix #2# line-prefix #8#) 485 502 (erc-timestamp 0 wrap-prefix #2# line-prefix #10=(space :width (- 27 (4)))) 502 503 (line-spacing 0.5 wrap-prefix #2# line-prefix #10#) 503 504 (erc-timestamp 0 wrap-prefix #2# line-prefix #9=(space :width (- 27 (6))) erc-command PRIVMSG) 504 507 (erc-timestamp 0 wrap-prefix #2# line-prefix #9# erc-command PRIVMSG) 507 525 (erc-timestamp 0 wrap-prefix #2# line-prefix #9# erc-command PRIVMSG) 525 526 (wrap-prefix #2# line-prefix #9#)) \ No newline at end of file -- 2.40.0
0003-5.6-Make-some-ERC-module-toggles-more-resilient.patch
(text/x-patch, 8.1 KB)
From 4a7391f478c58f46e02fa2e38693e9c60a0c679b Mon Sep 17 00:00:00 2001 From: "F. Jason Park" <[email protected]> Date: Sun, 7 May 2023 19:43:57 -0700 Subject: [PATCH 3/3] [5.6] Make some ERC module toggles more resilient * lisp/erc/erc-goodies.el (erc-scrolltobottom-mode, erc-scrolltobottom-enable, erc-move-to-prompt-mode, erc-move-to-prompt-enable): Guard setup procedure behind `erc--updating-modules-p'. * lisp/erc/erc-imenu.el (erc-imenu-mode, erc-imenu-enable, erc-imenu-disable): Don't run setup when `erc--updating-modules-p' is non-nil. Also don't restrict buffers to those of process on teardown. * lisp/erc/erc-match.el (erc-match-mode, erc-match-enable, erc-match-disable): Also major-mode hookee immediately. * lisp/erc/erc-spelling.el (erc-spelling-mode, erc-spelling-enable): Only conditionally run setup immediately. * lisp/erc/erc-stamp.el (erc-stamp-mode, erc-stamp-enable, erc-stamp-disable): Also run setup hook immediately. Don't forget to kill local vars in all ERC buffers during teardown. * lisp/erc/erc.el (erc--updating-modules-p): New variable that global modules use to provide their `erc-mode-hook'-deferred code on demand while guarding it from running during ERC buffer initialization. (erc-open): Make `erc--updating-modules-p' non-nil while activating global modules. --- lisp/erc/erc-goodies.el | 10 ++++------ lisp/erc/erc-imenu.el | 5 +++-- lisp/erc/erc-match.el | 4 +++- lisp/erc/erc-spelling.el | 5 +++-- lisp/erc/erc-stamp.el | 10 ++++++++-- lisp/erc/erc.el | 23 ++++++++++++++++++++++- 6 files changed, 43 insertions(+), 14 deletions(-) diff --git a/lisp/erc/erc-goodies.el b/lisp/erc/erc-goodies.el index 4558ff7c076..01eae4b63c5 100644 --- a/lisp/erc/erc-goodies.el +++ b/lisp/erc/erc-goodies.el @@ -53,9 +53,8 @@ scrolltobottom "This mode causes the prompt to stay at the end of the window." ((add-hook 'erc-mode-hook #'erc-add-scroll-to-bottom) (add-hook 'erc-insert-done-hook #'erc-possibly-scroll-to-bottom) - (dolist (buffer (erc-buffer-list)) - (with-current-buffer buffer - (erc-add-scroll-to-bottom)))) + (unless erc--updating-modules-p + (erc-buffer-filter #'erc-add-scroll-to-bottom))) ((remove-hook 'erc-mode-hook #'erc-add-scroll-to-bottom) (remove-hook 'erc-insert-done-hook #'erc-possibly-scroll-to-bottom) (dolist (buffer (erc-buffer-list)) @@ -120,9 +119,8 @@ erc-make-read-only (define-erc-module move-to-prompt nil "This mode causes the point to be moved to the prompt when typing text." ((add-hook 'erc-mode-hook #'erc-move-to-prompt-setup) - (dolist (buffer (erc-buffer-list)) - (with-current-buffer buffer - (erc-move-to-prompt-setup)))) + (unless erc--updating-modules-p + (erc-buffer-filter #'erc-move-to-prompt-setup))) ((remove-hook 'erc-mode-hook #'erc-move-to-prompt-setup) (dolist (buffer (erc-buffer-list)) (with-current-buffer buffer diff --git a/lisp/erc/erc-imenu.el b/lisp/erc/erc-imenu.el index 526afd32249..9864d7c4042 100644 --- a/lisp/erc/erc-imenu.el +++ b/lisp/erc/erc-imenu.el @@ -138,9 +138,10 @@ erc-imenu-setup ;;;###autoload(autoload 'erc-imenu-mode "erc-imenu" nil t) (define-erc-module imenu nil "Simple Imenu integration for ERC." - ((add-hook 'erc-mode-hook #'erc-imenu-setup)) + ((add-hook 'erc-mode-hook #'erc-imenu-setup) + (unless erc--updating-modules-p (erc-buffer-filter #'erc-imenu-setup))) ((remove-hook 'erc-mode-hook #'erc-imenu-setup) - (erc-with-all-buffers-of-server erc-server-process nil + (erc-with-all-buffers-of-server nil nil (when erc-imenu--create-index-function (setq imenu-create-index-function erc-imenu--create-index-function) (kill-local-variable 'erc-imenu--create-index-function))))) diff --git a/lisp/erc/erc-match.el b/lisp/erc/erc-match.el index c08a640260c..967ba039102 100644 --- a/lisp/erc/erc-match.el +++ b/lisp/erc/erc-match.el @@ -54,10 +54,12 @@ match highlighted." ((add-hook 'erc-insert-modify-hook #'erc-match-message 'append) (add-hook 'erc-mode-hook #'erc-match--modify-invisibility-spec) + (unless erc--updating-modules-p + (erc-buffer-filter #'erc-match--modify-invisibility-spec)) (erc--modify-local-map t "C-c C-k" #'erc-go-to-log-matches-buffer)) ((remove-hook 'erc-insert-modify-hook #'erc-match-message) (remove-hook 'erc-mode-hook #'erc-match--modify-invisibility-spec) - (erc-match--modify-invisibility-spec) + (erc-buffer-filter #'erc-match--modify-invisibility-spec) (erc--modify-local-map nil "C-c C-k" #'erc-go-to-log-matches-buffer))) ;; Remaining customizations diff --git a/lisp/erc/erc-spelling.el b/lisp/erc/erc-spelling.el index 8fce2508ceb..8e5424f4162 100644 --- a/lisp/erc/erc-spelling.el +++ b/lisp/erc/erc-spelling.el @@ -39,8 +39,9 @@ spelling ;; Use erc-connect-pre-hook instead of erc-mode-hook as pre-hook is ;; called AFTER the server buffer is initialized. ((add-hook 'erc-connect-pre-hook #'erc-spelling-init) - (dolist (buffer (erc-buffer-list)) - (erc-spelling-init buffer))) + (unless erc--updating-modules-p + (erc-with-all-buffers-of-server nil nil + (erc-spelling-init (current-buffer))))) ((remove-hook 'erc-connect-pre-hook #'erc-spelling-init) (dolist (buffer (erc-buffer-list)) (with-current-buffer buffer (flyspell-mode 0))))) diff --git a/lisp/erc/erc-stamp.el b/lisp/erc/erc-stamp.el index f90a8fc50b1..9191bbe5a2a 100644 --- a/lisp/erc/erc-stamp.el +++ b/lisp/erc/erc-stamp.el @@ -165,11 +165,17 @@ stamp ((add-hook 'erc-mode-hook #'erc-munge-invisibility-spec) (add-hook 'erc-insert-modify-hook #'erc-add-timestamp t) (add-hook 'erc-send-modify-hook #'erc-add-timestamp t) - (add-hook 'erc-mode-hook #'erc-stamp--recover-on-reconnect)) + (add-hook 'erc-mode-hook #'erc-stamp--recover-on-reconnect) + (unless erc--updating-modules-p + (erc-buffer-filter #'erc-munge-invisibility-spec))) ((remove-hook 'erc-mode-hook #'erc-munge-invisibility-spec) (remove-hook 'erc-insert-modify-hook #'erc-add-timestamp) (remove-hook 'erc-send-modify-hook #'erc-add-timestamp) - (remove-hook 'erc-mode-hook #'erc-stamp--recover-on-reconnect))) + (remove-hook 'erc-mode-hook #'erc-stamp--recover-on-reconnect) + (erc-with-all-buffers-of-server nil nil + (kill-local-variable 'erc-timestamp-last-inserted) + (kill-local-variable 'erc-timestamp-last-inserted-left) + (kill-local-variable 'erc-timestamp-last-inserted-right)))) (defun erc-stamp--recover-on-reconnect () (when-let ((priors (or erc--server-reconnecting erc--target-priors))) diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el index 249b9963cea..41d4f068ecd 100644 --- a/lisp/erc/erc.el +++ b/lisp/erc/erc.el @@ -2084,6 +2084,26 @@ erc--update-modules (push mode local-modes)) (error "`%s' is not a known ERC module" module))))) +(defvar erc--updating-modules-p nil + "Non-nil when running `erc--update-modules' in `erc-open'. +This allows global modules with known or likely dependents (or +some other reason for activating after session initialization) to +conditionally run setup code traditionally reserved for +`erc-mode-hook' in the setup portion of their mode toggle. Note +that being \"global\", they'll likely want to do so in all ERC +buffers and ensure the code is idempotent. For example: + + (add-hook \\='erc-mode-hook #\\='erc-foo-setup-fn) + (unless erc--updating-modules-p + (erc-with-all-buffers-of-server nil + (lambda () some-condition-p) + (erc-foo-setup-fn))) + +This means that when a dependent module is initializing and +realizes it's missing some required module \"foo\", it can +confidently call (erc-foo-mode 1) without having to learn +anything about the dependency's implementation.") + (defun erc--setup-buffer-first-window (frame a b) (catch 'found (walk-window-tree @@ -2243,7 +2263,8 @@ erc-open (set-buffer buffer) (setq old-point (point)) (setq delayed-modules - (erc--merge-local-modes (erc--update-modules) + (erc--merge-local-modes (let ((erc--updating-modules-p t)) + (erc--update-modules)) (or erc--server-reconnecting erc--target-priors))) -- 2.40.0