purcell / page-break-lines

Emacs: display ugly ^L page breaks as tidy horizontal lines
229 stars 31 forks source link

Line is one character too long with display-line-numbers-mode #37

Closed timhillgit closed 7 months ago

timhillgit commented 1 year ago

macOS version: 12.6 (21G115) Emacs version: GNU Emacs 28.2 (build 1, aarch64-apple-darwin21.1.0, NS appkit-2113.00 Version 12.0.1 (Build 21A559)) page-break-lines version: 20210104.2224

Steps to reproduce:

  1. Install emacs brew install --cask emacs
  2. Install page-break-lines (via melpa)
  3. emacs -q or emacs -q -nw
  4. M-x package-initialize
  5. M-x global-page-break-lines-mode
  6. M-x global-display-line-numbers-mode
  7. C-h f page-break-lines-mode
  8. Click page-break-lines.el
  9. The page break lines right above and below the function definition are one character too long.

Interestingly, replace steps 7–9 with the following for a counter example:

  1. M-x find-function RET page-break-lines-mode
  2. All page break lines in the file are the correct length.

I know there have been a lot of off-by-one issues that ended up not actually being bugs, let me know if any more information is helpful.

purcell commented 1 year ago

I can't reproduce this, on exactly the same Emacs and OS version. Note that when you call package-initialize, any number of other packages could still end up having an effect, even though you launched emacs with -q.

timhillgit commented 1 year ago

Hmm, that's odd! I was able to reproduce on a different laptop with a fresh install of emacs without using package-initialize by downloading page-break-lines.el and loading it directly. It may be some other OS setting I've set in both places? Give me a bit to do a screen recording as well, maybe I'm doing something without realizing it?

purcell commented 1 year ago

And you see it with both GUI mode and non-GUI mode? I'll try a bit harder to reproduce when I get some time.

timhillgit commented 1 year ago

I see it on both, though looking again in the non-GUI case it might be a fringe issue rather than an off-by-one issue. I edited page-break-lines.el to add some trace-values statements on intermediate values and ran a failing test case on GUI, a non-failing case on GUI, and a failing case on console. New page-break-lines: page-break-lines.txt

Failing GUI:

bad-gui

trace-output-bad.txt

Not-failing GUI

good-gui

trace-output-good.txt

Failing console:

bad-console

trace-output-console.txt

purcell commented 1 year ago

@shynur was also reporting this in #38. I still can't reproduce this locally. Could it possibly be font-specific? If someone could share what font they're using, I can test that out locally and perhaps I'll get the same result.

shynur commented 1 year ago

@purcell :

Could it possibly be font-specific?

I use Consolas, Cascadia, and Maple Mono. This issue arises when using either of them.

So perhaps it is because of something else. I will report further useful information if I find.

purcell commented 1 year ago

I have Consolas already, and that seems to work fine for me too. I've tried playing around with different orders of enabling modes etc. and just can't produce the same results.

timhillgit commented 1 year ago

I'm not changing any fonts when I use -q so I believe it would be using the default of Menlo. The only thing I can think of off the top of my head that I'm changing font-wise is that I've run defaults -currentHost write -g AppleFontSmoothing -int 0 to turn off font smoothing on my machine. I have a hard time believing that would be the cause, but it would be interesting if it was!

I haven't looked at this in a bit though, so I'll give it another look this weekend to see if anything has changed. My normal typeface is Iosevka and I'll try that as well.

jakewilliami commented 12 months ago

Hi team. I've reproduced this bug on two different computers, and made a minimum reproducible example .emacs file:

(require 'page-break-lines)
(global-page-break-lines-mode)
(global-display-line-numbers-mode)

Opening a file with ^L in it, with the above init configuration, will produce the bug. However, typing M-x C-g or M-SPC seems to force refresh the buffer and correct the bug goes away. This suggests that the bug may be related to the order in which this package is loaded. Perhaps there's another hook we need to listen for when loading.

Let me know if you can reproduce the issue with these steps, @purcell. I'll let you know if I discover anything else.

purcell commented 12 months ago

A variationn of this reciped let to reproduce the issue, at least when using a vanilla emacs 29.1 started with -Q, and using the init.el from my emacs config as the "file with ^L in it".

Near the top of the file, things work fine:

Screenshot 2023-10-16 at 08 03 32

But as I scroll farther, at around line 60, an additional space surprisingly appears before the still-two-digit-long line numbers:

Screenshot 2023-10-16 at 08 03 50

and then scrolling farther until a ^L comes into view, the page break line length has not been recomputed:

Screenshot 2023-10-16 at 08 04 07

display-line-numbers-mode hooks both pre-command-hook and window-scroll-hook so I'll have to think about how page-break-lines-mode might monitor changes in the line number width in a way that isn't dependent upon which of page-break-lines-mode and display-line-numbers-mode is enabled first.

timhillgit commented 7 months ago

With the new updates to how line width is computed this appears almost solved. I don't see the problem any longer with a GUI (macOS). On the terminal I can still reproduce it with the original steps, but it fixes as soon as I scroll and I don't typically use line numbers when I'm in terminal mode so it's a non-issue. Between this and page-break-lines-max-width you can consider this closed from my perspective unless you're still hunting it down. Thank you for all of your work @purcell !

purcell commented 7 months ago

Thanks Tim, I'd also consider this fixed now, or at least as good as it's likely to get. So I'll close this issue and folks can open a fresh one if they find a specific problem in future.