bug#45748: 28.0.50; fit-frame-to-buffer ignores leading spaces

[Aaron Jensen <aaronjensen@gmail.com>]

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

On Sat, Jan 9, 2021 at 12:14 PM Eli Zaretskii <eliz@gnu.org> wrote:
> Yes, probably.
>
> But before we do any changes here, we need a test suite.  Would you
> mind adding the necessary tests to test/src/xdisp-tests.el?

Okay, I added a few basic tests for this case. I don't know if that's
the best way to test it, but it's what I could figure out.

The implementation bypasses the extra backtrack for the FROM, but the
algorithm for the TO is the same as I first submitted. In that case,
it's not really an extra backtrack since I have to dec to read the
byte. If I had access to dec_bytepos declared in syntax.c, I think I
could use that to avoid the extra backtrack (I'd fetch the dec_bytepos
(bpos)) and test that and then only move the pointers if not breaking
from the loop.

I left the fetch_char_advance in the TO algorithm since it didn't seem
necessary to replace it with separate fetch/inc_both.

This could be done w/o the backtracking entirely by keeping two
pointers, but that's more complicated, probably not much more
efficient (if at all) and this will likely never be called in a tight
loop.

Let me know how this looks and if you want me to make any tweaks.

[-- Attachment #2: 0001-Fix-window-text-pixel-size-with-leading-trailing-spa.patch --]
[-- Type: application/octet-stream, Size: 3434 bytes --]

From 595937263d77bf392a32961119e0ce7f8754525b Mon Sep 17 00:00:00 2001
From: Aaron Jensen <aaronjensen@gmail.com>
Date: Sat, 9 Jan 2021 20:43:32 -0600
Subject: [PATCH] Fix window-text-pixel-size with leading/trailing spaces
 (bug#45748)

First, scan to find the first non-whitespace character and then
backtrack to find the beginning of the line. The previous algorithm
always started on the non-whitespace character during the backtrack,
causing it to stop immediately and not actually find the beginning of
the line. The same applies to the end of line calculation.

* src/xdisp.c: (Fwindow_text_pixel_size): Fix off by one
* test/src/xdisp-tests.el (xdisp-tests--window-text-pixel-size): New test
(xdisp-tests--window-text-pixel-size-leading-space): New test
(xdisp-tests--window-text-pixel-size-trailing-space): New test
---
 src/xdisp.c             |  8 ++++++--
 test/src/xdisp-tests.el | 30 ++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/src/xdisp.c b/src/xdisp.c
index 6a4304d194..6c8bd346c0 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -10650,9 +10650,10 @@ DEFUN ("window-text-pixel-size", Fwindow_text_pixel_size, Swindow_text_pixel_siz
       bpos = BEGV_BYTE;
       while (bpos < ZV_BYTE)
 	{
-	  c = fetch_char_advance (&start, &bpos);
+	  c = FETCH_BYTE (bpos);
 	  if (!(c == ' ' || c == '\t' || c == '\n' || c == '\r'))
 	    break;
+	  inc_both (&start, &bpos);
 	}
       while (bpos > BEGV_BYTE)
 	{
@@ -10681,7 +10682,10 @@ DEFUN ("window-text-pixel-size", Fwindow_text_pixel_size, Swindow_text_pixel_siz
 	  dec_both (&end, &bpos);
 	  c = FETCH_BYTE (bpos);
 	  if (!(c == ' ' || c == '\t' || c == '\n' || c == '\r'))
-	    break;
+            {
+	      inc_both (&end, &bpos);
+	      break;
+            }
 	}
       while (bpos < ZV_BYTE)
 	{
diff --git a/test/src/xdisp-tests.el b/test/src/xdisp-tests.el
index d13ce77a99..ec96d777ff 100644
--- a/test/src/xdisp-tests.el
+++ b/test/src/xdisp-tests.el
@@ -72,4 +72,34 @@ xdisp-tests--minibuffer-scroll
     (should (equal (nth 0 posns) (nth 1 posns)))
     (should (equal (nth 1 posns) (nth 2 posns)))))
 
+(ert-deftest xdisp-tests--window-text-pixel-size () ;; bug#45748
+  (with-temp-buffer
+    (insert "xxx")
+    (let* ((window
+            (display-buffer (current-buffer) '(display-buffer-in-child-frame . nil)))
+          (char-width (frame-char-width))
+          (size (window-text-pixel-size nil t t)))
+      (delete-frame (window-frame window))
+      (should (equal (/ (car size) char-width) 3)))))
+
+(ert-deftest xdisp-tests--window-text-pixel-size-leading-space () ;; bug#45748
+  (with-temp-buffer
+    (insert " xx")
+    (let* ((window
+            (display-buffer (current-buffer) '(display-buffer-in-child-frame . nil)))
+          (char-width (frame-char-width))
+          (size (window-text-pixel-size nil t t)))
+      (delete-frame (window-frame window))
+      (should (equal (/ (car size) char-width) 3)))))
+
+(ert-deftest xdisp-tests--window-text-pixel-size-trailing-space () ;; bug#45748
+  (with-temp-buffer
+    (insert "xx ")
+    (let* ((window
+            (display-buffer (current-buffer) '(display-buffer-in-child-frame . nil)))
+          (char-width (frame-char-width))
+          (size (window-text-pixel-size nil t t)))
+      (delete-frame (window-frame window))
+      (should (equal (/ (car size) char-width) 3)))))
+
 ;;; xdisp-tests.el ends here
-- 
2.28.0