GNU bug report logs - #44140
26.3; ERC stamps: Really use latest buffer's window's width prior to `fill-column'

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 44140 in the body.
You can then email your comments to 44140 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Olivier Certner <olivier.certner <at> free.fr>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.3; ERC stamps: Really use latest buffer's window's width prior to
 `fill-column'
Date: Thu, 22 Oct 2020 15:25:40 +0200

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

Message #8 received at 44140 <at> debbugs.gnu.org (full text, mbox):

From: Olivier Certner <ocert.dev <at> free.fr>
To: 44140 <at> debbugs.gnu.org
Subject: Patch
Date: Thu, 22 Oct 2020 17:22:24 +0200

[Message part 1 (text/plain, inline)]

In attachment.

-- 
Olivier Certner

[0001-ERC-stamps-Use-latest-buffer-s-window-s-width-prior-.patch (text/x-patch, attachment)]

Message #13 received at 44140 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Olivier Certner <olivier.certner <at> free.fr>
Cc: 44140 <at> debbugs.gnu.org, emacs-erc <at> gnu.org
Subject: Re: bug#44140: 26.3; ERC stamps: Really use latest buffer's
 window's width prior to `fill-column'
Date: Tue, 08 Jun 2021 20:56:12 -0700

[Message part 1 (text/plain, inline)]

Hi Olivier,

Olivier Certner <olivier.certner <at> 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.

[0001-ERC-stamps-Use-latest-buffer-s-window-s-width-prior-.patch (text/x-patch, attachment)]

[0002-Use-dynamic-align-to-spec-for-ERC-right-stamp.patch (text/x-patch, attachment)]

Message #16 received at 44140 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Olivier Certner <olivier.certner <at> free.fr>
Cc: 44140 <at> debbugs.gnu.org, emacs-erc <at> gnu.org
Subject: Re: bug#44140: 26.3; ERC stamps: Really use latest buffer's
 window's width prior to `fill-column'
Date: Tue, 08 Jun 2021 22:29:17 -0700

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.

Message #19 received at 44140 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Olivier Certner <olivier.certner <at> free.fr>
Cc: 44140 <at> debbugs.gnu.org, emacs-erc <at> gnu.org
Subject: Re: bug#44140: 26.3; ERC stamps: Really use latest buffer's
 window's width prior to `fill-column'
Date: Wed, 09 Jun 2021 02:31:17 -0700

"J.P." <jp <at> 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.

Message #22 received at 44140 <at> debbugs.gnu.org (full text, mbox):

From: Olivier Certner <olce.emacs <at> certner.fr>
To: 44140 <at> debbugs.gnu.org
Subject: Updated patch
Date: Tue, 06 Jul 2021 14:09:07 +0200

[Message part 1 (text/plain, inline)]

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

[0001-ERC-right-stamps-Use-also-latest-buffer-s-window-s-w.patch (text/x-patch, attachment)]

Message #25 received at 44140 <at> debbugs.gnu.org (full text, mbox):

From: Olivier Certner <olivier.certner <at> free.fr>
To: "J.P." <jp <at> neverwas.me>
Cc: 44140 <at> debbugs.gnu.org, emacs-erc <at> gnu.org
Subject: Re: bug#44140: 26.3;
 ERC stamps: Really use latest buffer's window's width prior to
 `fill-column'
Date: Tue, 06 Jul 2021 17:15:34 +0200

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

Message #28 received at 44140 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Olivier Certner <olivier.certner <at> free.fr>
Cc: 44140 <at> debbugs.gnu.org, emacs-erc <at> gnu.org
Subject: Re: bug#44140: 26.3; ERC stamps: Really use latest buffer's
 window's width prior to `fill-column'
Date: Wed, 07 Jul 2021 05:28:11 -0700

Olivier Certner <olivier.certner <at> 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.

Message #33 received at 44140-done <at> debbugs.gnu.org (full text, mbox):

From: Amin Bandali <bandali <at> gnu.org>
To: Olivier Certner <olce.emacs <at> certner.fr>
Cc: 44140-done <at> debbugs.gnu.org, "J.P." <jp <at> neverwas.me>
Subject: Re: bug#44140: 26.3; ERC stamps: Really use latest buffer's
 window's width prior to `fill-column'
Date: Fri, 06 Aug 2021 01:28:55 -0400

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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.