unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#44140: 26.3; ERC stamps: Really use latest buffer's window's width prior to `fill-column'
@ 2020-10-22 13:25 Olivier Certner
  2020-10-22 15:22 ` bug#44140: Patch Olivier Certner
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Olivier Certner @ 2020-10-22 13:25 UTC (permalink / raw)
  To: 44140

Applies to 26.3, but also all more recent versions as well.

Bug trigger:
1. Load ERC (e.g., open some IRC connection).
2. In some ERC buffer, deactivate ERC fill mode (if not deactivated
globally in your configuration), e.g., M-x erc-fill-mode.
3. Notice that `fill-column' (if non-nil; if nil, set it temporarily to
some small value, e.g., 40, to see the effect) will be used in this
case, contrary to what the documentation of `erc-insert-timestamp-right'
says (window's width is supposed to be used in priority when
erc-fill-mode is not activated).
4. Set `fill-column' to nil. Then, move to another window selecting
another buffer, whose size is visibly smaller. Wait for messages to
arrive in the ERC buffer and observe the timestamp position: It is
relative to the currently selected window's width!

Root causes:
1. Precedence of window's width over `fill-column' not implemented,
contrary to what the documentation states, which makes much more sense.
2. `window-width' is called to obtain window's width, but this gets the
width of the selected window, which is not necessarily where the buffer
is actually displayed. Moreover, the buffer may not be displayed at the
moment, so some other fallback is necessary.

Patch:
To be attached shortly after bug creation.

-- 
Olivier Certner







^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#44140: Patch
  2020-10-22 13:25 bug#44140: 26.3; ERC stamps: Really use latest buffer's window's width prior to `fill-column' Olivier Certner
@ 2020-10-22 15:22 ` Olivier Certner
  2021-06-09  3:56 ` bug#44140: 26.3; ERC stamps: Really use latest buffer's window's width prior to `fill-column' J.P.
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Olivier Certner @ 2020-10-22 15:22 UTC (permalink / raw)
  To: 44140

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

In attachment.

-- 
Olivier Certner

[-- Attachment #2: 0001-ERC-stamps-Use-latest-buffer-s-window-s-width-prior-.patch --]
[-- Type: text/x-patch, Size: 6288 bytes --]

From 33b1c2d7b7574a70591f27c6718dffb29503b2de Mon Sep 17 00:00:00 2001
From: Olivier Certner <ocert.dev@free.fr>
Date: Thu, 22 Oct 2020 12:01:26 +0200
Subject: [PATCH] ERC stamps: Use latest buffer's window's width prior to
 `fill-column'

* lisp/erc/erc-stamp.el (erc-insert-timestamp-right): Use latest
buffer's window's width to position the timestamp, if both
`erc-timestamp-right-column' and `erc-fill-column' are not set (or
`erc-fill-mode' is off).  This is what the documentation says, but was
not implemented.  Also fix the bug of using selected window's width
instead of the (or some) window showing the buffer.  The latest
window's width is saved in `erc-timestamp-last-window-width' and used
when the buffer is no more shown.  In case the buffer was never
showed, which I'm not sure can happen, either use `fill-column' if
set, or give up on aligning and just output the timestamp (modulo the
kludge) right after each the text on the concerned line.  While here,
fix the off by one calculation of point start when the reference is
the window's width.
---
 ChangeLog.3           | 19 +++++++++++++++++
 lisp/erc/erc-stamp.el | 48 ++++++++++++++++++++++++++++---------------
 2 files changed, 50 insertions(+), 17 deletions(-)

diff --git a/ChangeLog.3 b/ChangeLog.3
index ec2d3f8d46..c040fa889e 100644
--- a/ChangeLog.3
+++ b/ChangeLog.3
@@ -1,3 +1,22 @@
+2020-10-22  Olivier Certner  <ocert.dev@free.fr>
+
+	ERC stamps: Use latest buffer's window's width prior to `fill-column'
+
+	* lisp/erc/erc-stamp.el (erc-insert-timestamp-right): Use latest
+	buffer's window's width to position the timestamp, if both
+	`erc-timestamp-right-column' and `erc-fill-column' are not set (or
+	`erc-fill-mode' is off).  This is what the documentation says, but
+	was not implemented.  Also fix the bug of using selected window's
+	width instead of the (or some) window showing the buffer.  The
+	latest window's width is saved in
+	`erc-timestamp-last-window-width' and used when the buffer is no
+	more shown.  In case the buffer was never showed, which I'm not
+	sure can happen, either use `fill-column' if set, or give up on
+	aligning and just output the timestamp (modulo the kludge) right
+	after each the text on the concerned line.  While here, fix the
+	off by one calculation of point start when the reference is the
+	window's width.
+
 2020-08-03  Phil Sainty  <psainty@orcon.net.nz>
 
 	lisp/so-long.el: Improve support for major mode hooks
diff --git a/lisp/erc/erc-stamp.el b/lisp/erc/erc-stamp.el
index 08970f2d70..437b428e03 100644
--- a/lisp/erc/erc-stamp.el
+++ b/lisp/erc/erc-stamp.el
@@ -191,6 +191,11 @@ or `erc-send-modify-hook'."
 				 (list (lambda (_window _before dir)
 					 (erc-echo-timestamp dir ct))))))))
 
+(defvar-local erc-timestamp-last-window-width nil
+  "Stores the width of the last window that showed the current
+buffer. This is used by `erc-insert-timestamp-right' when the
+current buffer is not shown in any window.")
+
 (defvar erc-timestamp-last-inserted nil
   "Last timestamp inserted into the buffer.")
 (make-variable-buffer-local 'erc-timestamp-last-inserted)
@@ -266,27 +271,32 @@ property to get to the POSth column."
 
 (defun erc-insert-timestamp-right (string)
   "Insert timestamp on the right side of the screen.
-STRING is the timestamp to insert.  The function is a possible value
-for `erc-insert-timestamp-function'.
-
-If `erc-timestamp-only-if-changed-flag' is nil, a timestamp is always
-printed.  If this variable is non-nil, a timestamp is only printed if
-it is different from the last.
-
-If `erc-timestamp-right-column' is set, its value will be used as the
-column at which the timestamp is to be printed.  If it is nil, and
-`erc-fill-mode' is active, then the timestamp will be printed just
-before `erc-fill-column'.  Otherwise, if the current buffer is
-shown in a window, that window's width is used.  If the buffer is
-not shown, and `fill-column' is set, then the timestamp will be
-printed just `fill-column'.  As a last resort, the timestamp will
-be printed just before the window-width."
+STRING is the timestamp to insert.  The function is a possible
+value for `erc-insert-timestamp-function'.
+
+If `erc-timestamp-only-if-changed-flag' is nil, a timestamp is
+always printed.  If this variable is non-nil, a timestamp is only
+printed if it is different from the last.
+
+If `erc-timestamp-right-column' is set, its value will be used as
+the column at which the timestamp is to be printed.  If it is
+nil, and `erc-fill-mode' is active, then the timestamp will be
+printed just before `erc-fill-column'.  Otherwise, if the current
+buffer is shown in a window, that window's width is used.  In
+case multiple windows show the buffer, the width of the most
+recently selected one is used.  If the buffer is not shown, the
+timestamp will be printed just before the window width of the
+last window that showed it.  If the buffer was never shown, and
+`fill-column' is set, it will be printed just before
+`fill-column'.  As a last resort, timestamp will be printed just
+after each line's text (no alignment)."
   (unless (and erc-timestamp-only-if-changed-flag
 	       (string-equal string erc-timestamp-last-inserted))
     (setq erc-timestamp-last-inserted string)
     (goto-char (point-max))
     (forward-char -1);; before the last newline
     (let* ((str-width (string-width string))
+           window ; Used in 'pos' computation only
 	   (pos (cond
 		 (erc-timestamp-right-column erc-timestamp-right-column)
 		 ((and (boundp 'erc-fill-mode)
@@ -294,10 +304,14 @@ be printed just before the window-width."
 		       (boundp 'erc-fill-column)
 		       erc-fill-column)
 		  (1+ (- erc-fill-column str-width)))
+                 ((setq window (get-buffer-window nil t))
+                  (setq erc-timestamp-last-window-width (window-width window))
+                  (- erc-timestamp-last-window-width str-width))
+                 (erc-timestamp-last-window-width
+		  (- erc-timestamp-last-window-width str-width))
 		 (fill-column
 		  (1+ (- fill-column str-width)))
-		 (t
-		  (- (window-width) str-width 1))))
+                 (t (current-column))))
 	   (from (point))
 	   (col (current-column)))
       ;; The following is a kludge used to calculate whether to move
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* bug#44140: 26.3; ERC stamps: Really use latest buffer's window's width prior to `fill-column'
  2020-10-22 13:25 bug#44140: 26.3; ERC stamps: Really use latest buffer's window's width prior to `fill-column' Olivier Certner
  2020-10-22 15:22 ` bug#44140: Patch Olivier Certner
@ 2021-06-09  3:56 ` J.P.
  2021-06-09  5:29 ` J.P.
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: J.P. @ 2021-06-09  3:56 UTC (permalink / raw)
  To: Olivier Certner; +Cc: 44140, emacs-erc

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

Hi Olivier,

Olivier Certner <olivier.certner@free.fr> writes:

> 4. Set `fill-column' to nil. Then, move to another window selecting
> another buffer, whose size is visibly smaller. Wait for messages to
> arrive in the ERC buffer and observe the timestamp position: It is
> relative to the currently selected window's width!

I followed the steps and observed the effects as you describe. Setting
fill-column to nil (which I wasn't aware was possible) makes it use the
width of whichever window's currently active. Definitely worthy of a "!".

> 2. `window-width' is called to obtain window's width, but this gets the
> width of the selected window, which is not necessarily where the buffer
> is actually displayed. Moreover, the buffer may not be displayed at the
> moment, so some other fallback is necessary.

Storing the last window's width seems like a good idea because these
buffers are often buried.

I'm a little fuzzy on how the ALL-FRAMES = t param for the function
`get-buffer-window' works exactly. The windows within each frame should
follow the normal cyclic ordering (right?). But I think I learned
somewhere that frame ordering is different and isn't affected by
whichever one was last selected. If true, I suppose frame users (not me)
are already used to this behavior and won't be surprised by it.

Anyway, I happened upon another approach for the final display part (see
attached sketch). If you see anything useful, just take it. Otherwise,
sorry for the distraction.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ERC-stamps-Use-latest-buffer-s-window-s-width-prior-.patch --]
[-- Type: text/x-patch, Size: 4935 bytes --]

From f10df51f8b65cf4e25a77eb52d3e3a0320e317a8 Mon Sep 17 00:00:00 2001
From: Olivier Certner <ocert.dev@free.fr>
Date: Thu, 22 Oct 2020 12:01:26 +0200
Subject: [PATCH 1/2] ERC stamps: Use latest buffer's window's width prior to
 `fill-column'

* lisp/erc/erc-stamp.el (erc-insert-timestamp-right): Use latest
buffer's window's width to position the timestamp, if both
`erc-timestamp-right-column' and `erc-fill-column' are not set (or
`erc-fill-mode' is off).  This is what the documentation says, but was
not implemented.  Also fix the bug of using selected window's width
instead of the (or some) window showing the buffer.  The latest
window's width is saved in `erc-timestamp-last-window-width' and used
when the buffer is no more shown.  In case the buffer was never
showed, which I'm not sure can happen, either use `fill-column' if
set, or give up on aligning and just output the timestamp (modulo the
kludge) right after each the text on the concerned line.  While here,
fix the off by one calculation of point start when the reference is
the window's width.
---
 lisp/erc/erc-stamp.el | 48 ++++++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/lisp/erc/erc-stamp.el b/lisp/erc/erc-stamp.el
index 31de9e80697..40457a1a2b4 100644
--- a/lisp/erc/erc-stamp.el
+++ b/lisp/erc/erc-stamp.el
@@ -181,6 +181,11 @@ erc-add-timestamp
 				 (list (lambda (_window _before dir)
 					 (erc-echo-timestamp dir ct))))))))
 
+(defvar-local erc-timestamp-last-window-width nil
+  "Stores the width of the last window that showed the current
+buffer. This is used by `erc-insert-timestamp-right' when the
+current buffer is not shown in any window.")
+
 (defvar-local erc-timestamp-last-inserted nil
   "Last timestamp inserted into the buffer.")
 
@@ -250,27 +255,32 @@ erc-fill-column
 
 (defun erc-insert-timestamp-right (string)
   "Insert timestamp on the right side of the screen.
-STRING is the timestamp to insert.  The function is a possible value
-for `erc-insert-timestamp-function'.
-
-If `erc-timestamp-only-if-changed-flag' is nil, a timestamp is always
-printed.  If this variable is non-nil, a timestamp is only printed if
-it is different from the last.
-
-If `erc-timestamp-right-column' is set, its value will be used as the
-column at which the timestamp is to be printed.  If it is nil, and
-`erc-fill-mode' is active, then the timestamp will be printed just
-before `erc-fill-column'.  Otherwise, if the current buffer is
-shown in a window, that window's width is used.  If the buffer is
-not shown, and `fill-column' is set, then the timestamp will be
-printed just `fill-column'.  As a last resort, the timestamp will
-be printed just before the window-width."
+STRING is the timestamp to insert.  The function is a possible
+value for `erc-insert-timestamp-function'.
+
+If `erc-timestamp-only-if-changed-flag' is nil, a timestamp is
+always printed.  If this variable is non-nil, a timestamp is only
+printed if it is different from the last.
+
+If `erc-timestamp-right-column' is set, its value will be used as
+the column at which the timestamp is to be printed.  If it is
+nil, and `erc-fill-mode' is active, then the timestamp will be
+printed just before `erc-fill-column'.  Otherwise, if the current
+buffer is shown in a window, that window's width is used.  In
+case multiple windows show the buffer, the width of the most
+recently selected one is used.  If the buffer is not shown, the
+timestamp will be printed just before the window width of the
+last window that showed it.  If the buffer was never shown, and
+`fill-column' is set, it will be printed just before
+`fill-column'.  As a last resort, timestamp will be printed just
+after each line's text (no alignment)."
   (unless (and erc-timestamp-only-if-changed-flag
 	       (string-equal string erc-timestamp-last-inserted))
     (setq erc-timestamp-last-inserted string)
     (goto-char (point-max))
     (forward-char -1);; before the last newline
     (let* ((str-width (string-width string))
+           window ; Used in 'pos' computation only
 	   (pos (cond
 		 (erc-timestamp-right-column erc-timestamp-right-column)
 		 ((and (boundp 'erc-fill-mode)
@@ -278,10 +288,14 @@ erc-insert-timestamp-right
 		       (boundp 'erc-fill-column)
 		       erc-fill-column)
 		  (1+ (- erc-fill-column str-width)))
+                 ((setq window (get-buffer-window nil t))
+                  (setq erc-timestamp-last-window-width (window-width window))
+                  (- erc-timestamp-last-window-width str-width))
+                 (erc-timestamp-last-window-width
+		  (- erc-timestamp-last-window-width str-width))
 		 (fill-column
 		  (1+ (- fill-column str-width)))
-		 (t
-		  (- (window-width) str-width 1))))
+                 (t (current-column))))
 	   (from (point))
 	   (col (current-column)))
       ;; The following is a kludge used to calculate whether to move
-- 
2.31.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Use-dynamic-align-to-spec-for-ERC-right-stamp.patch --]
[-- Type: text/x-patch, Size: 3481 bytes --]

From d006e8ef3de29d1e4374718eb909babc32d70090 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Tue, 8 Jun 2021 20:22:59 -0700
Subject: [PATCH 2/2] Use dynamic align-to spec for ERC right stamp

* erc-stamp.el (erc-timestamp-use-align-to,
erc-timestamp-align-to-gap): replace former with latter.  Assume
:align-to display property is always supported regardless of windowing
system.  New variable specifies space between right timestamp and
right margin.
(erc-insert-aligned): Always use display spec regardless of windowing
system. Make arg POS optional. When not provided, use dynamic offset.
(erc-fill-column): Only use computed position to determine whether to
insert newline before stamp.
---
 lisp/erc/erc-stamp.el | 39 +++++++++++++++------------------------
 1 file changed, 15 insertions(+), 24 deletions(-)

diff --git a/lisp/erc/erc-stamp.el b/lisp/erc/erc-stamp.el
index 40457a1a2b4..985bb785da3 100644
--- a/lisp/erc/erc-stamp.el
+++ b/lisp/erc/erc-stamp.el
@@ -217,14 +217,12 @@ erc-timestamp-right-column
 	  (integer :tag "Column number")
 	  (const :tag "Unspecified" nil)))
 
-(defcustom erc-timestamp-use-align-to (eq window-system 'x)
-  "If non-nil, use the :align-to display property to align the stamp.
-This gives better results when variable-width characters (like
-Asian language characters and math symbols) precede a timestamp.
+(defcustom erc-timestamp-align-to-gap 2
+  "Amount of space between right margin and right timestamp."
+  :type 'number)
 
-A side effect of enabling this is that there will only be one
-space before a right timestamp in any saved logs."
-  :type 'boolean)
+(make-obsolete-variable 'erc-timestamp-use-align-to
+                        'erc-timestamp-align-to-gap "28.1")
 
 (defun erc-insert-timestamp-left (string)
   "Insert timestamps at the beginning of the line."
@@ -238,17 +236,14 @@ erc-insert-timestamp-left
     (erc-put-text-property 0 len 'invisible 'timestamp s)
     (insert s)))
 
-(defun erc-insert-aligned (string pos)
-  "Insert STRING at the POSth column.
-
-If `erc-timestamp-use-align-to' is t, use the :align-to display
-property to get to the POSth column."
-  (if (not erc-timestamp-use-align-to)
-      (indent-to pos)
-    (insert " ")
-    (put-text-property (1- (point)) (point) 'display
-		       (list 'space ':align-to pos)))
-  (insert string))
+(defun erc-insert-aligned (string &optional pos)
+  "Insert STRING as right timestamp.
+Use POS when provided, otherwise use string's width and
+`erc-timestamp-align-to-gap' to determine position."
+  (let ((spec (or pos `(- right ,(+ erc-timestamp-align-to-gap
+                                    (string-width string))))))
+    (insert (propertize " " 'display `(space :align-to ,spec))
+            string)))
 
 ;; Silence byte-compiler
 (defvar erc-fill-column)
@@ -303,12 +298,8 @@ erc-insert-timestamp-right
       ;; some margin of error if what is displayed on the line differs
       ;; from the number of characters on the line.
       (setq col (+ col (ceiling (/ (- col (- (point) (point-at-bol))) 1.6))))
-      (if (< col pos)
-	  (erc-insert-aligned string pos)
-	(newline)
-	(indent-to pos)
-	(setq from (point))
-	(insert string))
+      (when (>= col pos) (newline))
+      (erc-insert-aligned string erc-timestamp-right-column)
       (erc-put-text-property from (point) 'field 'erc-timestamp)
       (erc-put-text-property from (point) 'rear-nonsticky t)
       (when erc-timestamp-intangible
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* bug#44140: 26.3; ERC stamps: Really use latest buffer's window's width prior to `fill-column'
  2020-10-22 13:25 bug#44140: 26.3; ERC stamps: Really use latest buffer's window's width prior to `fill-column' Olivier Certner
  2020-10-22 15:22 ` bug#44140: Patch Olivier Certner
  2021-06-09  3:56 ` bug#44140: 26.3; ERC stamps: Really use latest buffer's window's width prior to `fill-column' J.P.
@ 2021-06-09  5:29 ` J.P.
       [not found] ` <87v96n61xu.fsf@neverwas.me>
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: J.P. @ 2021-06-09  5:29 UTC (permalink / raw)
  To: Olivier Certner; +Cc: 44140, emacs-erc

Actually, you'd probably have to include that silly gap variable I added
in all the "should-newline-p" figuring. So maybe something like:

  -       (insert string))
  +      (when (>= col (- pos erc-timestamp-align-to-gap)) (newline))
                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  +      (erc-insert-aligned string erc-timestamp-right-column)

I don't know.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#44140: 26.3; ERC stamps: Really use latest buffer's window's width prior to `fill-column'
       [not found] ` <87v96n61xu.fsf@neverwas.me>
@ 2021-06-09  9:31   ` J.P.
  0 siblings, 0 replies; 9+ messages in thread
From: J.P. @ 2021-06-09  9:31 UTC (permalink / raw)
  To: Olivier Certner; +Cc: 44140, emacs-erc

"J.P." <jp@neverwas.me> writes:

> Actually, you'd probably have to include that silly gap variable I added

As well as something like

  @@ -303,12 +298,8 @@ erc-insert-timestamp-right
         ;; some margin of error if what is displayed on the line differs
         ;; from the number of characters on the line.
         (setq col (+ col (ceiling (/ (- col (- (point) (point-at-bol))) 1.6))))
  -      (if (< col pos)
  -         (erc-insert-aligned string pos)
  -       (newline)
  -       (indent-to pos)
  -       (setq from (point))
  -       (insert string))
  +      (when (>= col (- pos erc-timestamp-align-to-gap)) (newline))
  +      (erc-insert-aligned string (unless erc-timestamp-last-window-width pos))
                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         (erc-put-text-property from (point) 'field 'erc-timestamp)
         (erc-put-text-property from (point) 'rear-nonsticky t)
         (when erc-timestamp-intangible

to honor existing behavior when erc-fill-mode is active. (As well as
other common-sense stuff I'm surely missing.)

It also strikes me that some 'fill users might prefer only having
`erc-fill-mode' affect message text while having timestamps instead
aligned to a window's width. So, yet another option could be added to
make something like that a reality if you think there'd be sufficient
demand.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#44140: Updated patch
  2020-10-22 13:25 bug#44140: 26.3; ERC stamps: Really use latest buffer's window's width prior to `fill-column' Olivier Certner
                   ` (3 preceding siblings ...)
       [not found] ` <87v96n61xu.fsf@neverwas.me>
@ 2021-07-06 12:09 ` Olivier Certner
  2021-08-06  5:28   ` bug#44140: 26.3; ERC stamps: Really use latest buffer's window's width prior to `fill-column' Amin Bandali
       [not found] ` <87bl8f7ktf.fsf@neverwas.me>
  5 siblings, 1 reply; 9+ messages in thread
From: Olivier Certner @ 2021-07-06 12:09 UTC (permalink / raw)
  To: 44140

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

Patch slightly edited (commit message, doc text) and rebased on top of a 
recent 'master'. Ready to apply. Easy to backport to 27 as well.

-- 
Olivier Certner

[-- Attachment #2: 0001-ERC-right-stamps-Use-also-latest-buffer-s-window-s-w.patch --]
[-- Type: text/x-patch, Size: 4927 bytes --]

From 3658345e21d65c1c4e72ec7a7d90e0dcf08f1f7f Mon Sep 17 00:00:00 2001
From: Olivier Certner <olce.emacs@certner.fr>
Date: Tue, 6 Jul 2021 12:35:43 +0200
Subject: [PATCH] ERC right stamps: Use also latest buffer's window's width
 (Bug#44140)

* lisp/erc/erc-stamp.el (erc-insert-timestamp-right): Use latest
buffer's window's width to position the timestamp, if both
`erc-timestamp-right-column' and `erc-fill-column' are not set (or
`erc-fill-mode' is off).  This is what the documentation says, but was
not implemented.  Also fix the bug of using selected window's width
instead of the (or some) window showing the buffer.  The latest
window's width is saved in `erc-timestamp-last-window-width' and used
when the buffer is no more shown.  In case the buffer was never shown,
which I'm not sure can happen, either use `fill-column' if set, or
give up on aligning and just output the timestamp (modulo the kludge)
right after message text.  While here, fix the off by one calculation
of point start when the reference is the window's width.
---
 lisp/erc/erc-stamp.el | 44 ++++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/lisp/erc/erc-stamp.el b/lisp/erc/erc-stamp.el
index 31de9e8069..4cf744549d 100644
--- a/lisp/erc/erc-stamp.el
+++ b/lisp/erc/erc-stamp.el
@@ -181,6 +181,11 @@ erc-add-timestamp
 				 (list (lambda (_window _before dir)
 					 (erc-echo-timestamp dir ct))))))))
 
+(defvar-local erc-timestamp-last-window-width nil
+  "Stores the width of the last window that showed the current
+buffer. This is used by `erc-insert-timestamp-right' when the
+current buffer is not shown in any window.")
+
 (defvar-local erc-timestamp-last-inserted nil
   "Last timestamp inserted into the buffer.")
 
@@ -250,27 +255,32 @@ erc-fill-column
 
 (defun erc-insert-timestamp-right (string)
   "Insert timestamp on the right side of the screen.
-STRING is the timestamp to insert.  The function is a possible value
-for `erc-insert-timestamp-function'.
+STRING is the timestamp to insert.  This function is a possible
+value for `erc-insert-timestamp-function'.
 
-If `erc-timestamp-only-if-changed-flag' is nil, a timestamp is always
-printed.  If this variable is non-nil, a timestamp is only printed if
-it is different from the last.
+If `erc-timestamp-only-if-changed-flag' is nil, a timestamp is
+always printed.  If this variable is non-nil, a timestamp is only
+printed if it is different from the last.
 
-If `erc-timestamp-right-column' is set, its value will be used as the
-column at which the timestamp is to be printed.  If it is nil, and
-`erc-fill-mode' is active, then the timestamp will be printed just
-before `erc-fill-column'.  Otherwise, if the current buffer is
-shown in a window, that window's width is used.  If the buffer is
-not shown, and `fill-column' is set, then the timestamp will be
-printed just `fill-column'.  As a last resort, the timestamp will
-be printed just before the window-width."
+If `erc-timestamp-right-column' is set, its value will be used as
+the column at which the timestamp is to be printed.  If it is
+nil, and `erc-fill-mode' is active, then the timestamp will be
+printed just before `erc-fill-column'.  Otherwise, if the current
+buffer is shown in a window, that window's width is used as the
+right boundary.  In case multiple windows show the buffer, the
+width of the most recently selected one is used.  If the buffer
+is not shown, the timestamp will be printed just before the
+window width of the last window that showed it.  If the buffer
+was never shown, and `fill-column' is set, it will be printed
+just before `fill-column'.  As a last resort, timestamp will be
+printed just after each line's text (no alignment)."
   (unless (and erc-timestamp-only-if-changed-flag
 	       (string-equal string erc-timestamp-last-inserted))
     (setq erc-timestamp-last-inserted string)
     (goto-char (point-max))
     (forward-char -1);; before the last newline
     (let* ((str-width (string-width string))
+           window ; Used in 'pos' computation only
 	   (pos (cond
 		 (erc-timestamp-right-column erc-timestamp-right-column)
 		 ((and (boundp 'erc-fill-mode)
@@ -278,10 +288,14 @@ erc-insert-timestamp-right
 		       (boundp 'erc-fill-column)
 		       erc-fill-column)
 		  (1+ (- erc-fill-column str-width)))
+                 ((setq window (get-buffer-window nil t))
+                  (setq erc-timestamp-last-window-width (window-width window))
+                  (- erc-timestamp-last-window-width str-width))
+                 (erc-timestamp-last-window-width
+		  (- erc-timestamp-last-window-width str-width))
 		 (fill-column
 		  (1+ (- fill-column str-width)))
-		 (t
-		  (- (window-width) str-width 1))))
+                 (t (current-column))))
 	   (from (point))
 	   (col (current-column)))
       ;; The following is a kludge used to calculate whether to move
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* bug#44140: 26.3; ERC stamps: Really use latest buffer's window's width prior to `fill-column'
       [not found] ` <87bl8f7ktf.fsf@neverwas.me>
@ 2021-07-06 15:15   ` Olivier Certner
  2021-07-07 12:28     ` J.P.
  0 siblings, 1 reply; 9+ messages in thread
From: Olivier Certner @ 2021-07-06 15:15 UTC (permalink / raw)
  To: J.P.; +Cc: 44140, emacs-erc

Hi JP,

> I'm a little fuzzy on how the ALL-FRAMES = t param for the function
> `get-buffer-window' works exactly. The windows within each frame should
> follow the normal cyclic ordering (right?). But I think I learned
> somewhere that frame ordering is different and isn't affected by
> whichever one was last selected. If true, I suppose frame users (not me)
> are already used to this behavior and won't be surprised by it.

After reading some code (in "window.c"), I think `get-buffer-window' works 
like this:
1. It browses all windows in cyclic order (including windows of other frames 
or not, depending on the ALL-FRAMES parameter).
2. If the currently selected window contains the wanted buffer, it is returned 
immediately.
3. If 2 never occurs, and there is a window containing the current buffer in 
the selected frame, then the first one (i.e., the most recently activated) is 
returned.
4. If 2 and 3 never occur, than the first window containing the current buffer 
is returned (so, a window from another frame).
 
> Anyway, I happened upon another approach for the final display part (see
> attached sketch). If you see anything useful, just take it. Otherwise,
> sorry for the distraction.

Your changes seem interesting. I'm not very familiar with display properties, 
and I'm wondering if this would work as expected on text displays. Since I 
don't have much time to test that, and since these changes are independent of 
the bugs fixed here, I'd suggest to put them in a separate report.

Regards.

-- 
Olivier Certner







^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#44140: 26.3; ERC stamps: Really use latest buffer's window's width prior to `fill-column'
  2021-07-06 15:15   ` Olivier Certner
@ 2021-07-07 12:28     ` J.P.
  0 siblings, 0 replies; 9+ messages in thread
From: J.P. @ 2021-07-07 12:28 UTC (permalink / raw)
  To: Olivier Certner; +Cc: 44140, emacs-erc

Olivier Certner <olivier.certner@free.fr> writes:

> After reading some code (in "window.c"), I think `get-buffer-window' works 
> like this:
> 1. It browses all windows in cyclic order (including windows of other frames 
> or not, depending on the ALL-FRAMES parameter).
> 2. If the currently selected window contains the wanted buffer, it is returned 
> immediately.
> 3. If 2 never occurs, and there is a window containing the current buffer in 
> the selected frame, then the first one (i.e., the most recently activated) is 
> returned.
> 4. If 2 and 3 never occur, than the first window containing the current buffer 
> is returned (so, a window from another frame).

I ended up stepping through some of the underlying functions that
determine this behavior. It seems I failed to grasp/remember what
"cyclic ordering" meant when reviewing your patch initially. Shocking,
I know (cough).

It's now my "belief" that the most recently selected ("other"/"old")
window (or frame) does *not* impact selecting/visiting by these core
functions. New windows/frames are just consed onto the front of global
variables designated for this purpose. So the visiting order is set in
stone and starts from the next oldest after the querying entity, moving
toward the oldest. It then wraps around and goes from the youngest
(newest) back toward itself.

You likely had the right idea originally but were thrown off by my
stupidity re "most recently activated" in #3. Anyway, the takeaway is
that this behavior is predictable as long as people grok and expect
(info "(elisp) Cyclic Window Ordering").

> Your changes seem interesting. I'm not very familiar with display properties, 
> and I'm wondering if this would work as expected on text displays. Since I 
> don't have much time to test that, and since these changes are independent of 
> the bugs fixed here, I'd suggest to put them in a separate report.

If only there were folks familiar enough with display properties to
offer some guidance to little old ERC. Assuming no, then no matter; it's
not worth fussing over what's basically only a cosmetic concern when we
have bigger fish to fry.

Anyway, good job. This one's ready, I guess.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#44140: 26.3; ERC stamps: Really use latest buffer's window's width prior to `fill-column'
  2021-07-06 12:09 ` bug#44140: Updated patch Olivier Certner
@ 2021-08-06  5:28   ` Amin Bandali
  0 siblings, 0 replies; 9+ messages in thread
From: Amin Bandali @ 2021-08-06  5:28 UTC (permalink / raw)
  To: Olivier Certner; +Cc: 44140-done, J.P.

Hi Olivier, J.P.,

Olivier Certner writes:

> Patch slightly edited (commit message, doc text) and rebased on top of a 
> recent 'master'. Ready to apply. Easy to backport to 27 as well.

Thanks for the patch(es) and discussion.  Applied to emacs-27.





^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-08-06  5:28 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-22 13:25 bug#44140: 26.3; ERC stamps: Really use latest buffer's window's width prior to `fill-column' Olivier Certner
2020-10-22 15:22 ` bug#44140: Patch Olivier Certner
2021-06-09  3:56 ` bug#44140: 26.3; ERC stamps: Really use latest buffer's window's width prior to `fill-column' J.P.
2021-06-09  5:29 ` J.P.
     [not found] ` <87v96n61xu.fsf@neverwas.me>
2021-06-09  9:31   ` J.P.
2021-07-06 12:09 ` bug#44140: Updated patch Olivier Certner
2021-08-06  5:28   ` bug#44140: 26.3; ERC stamps: Really use latest buffer's window's width prior to `fill-column' Amin Bandali
     [not found] ` <87bl8f7ktf.fsf@neverwas.me>
2021-07-06 15:15   ` Olivier Certner
2021-07-07 12:28     ` 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).