bug#53375: [PATCH] Account for padding and content length in, tabulated-list-widen-current-column

bug#53375: [PATCH] Account for padding and content length in, tabulated-list-widen-current-column

[Thuna]

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


`tabulated-list-widen-current-column' previously looked only at the
width of the column as given in `tabulated-list-format' however this
fails to account for both the padding and content length and as a
result widens the wrong column.

The issue with content length is reproducible as follows:
0. emacs -Q
1. M-x shell
2. M-x list-processes
3. C-x o
4. C-: (tabulated-list-set-col 0 (make-string 50 ?.) t)
5. M-f
6. }


This patch should solve the issue however it relies on
`tabulated-list-get-entry' which means it fails to work if the
`tabulated-list-entry' property is not set properly.  I am unsure if
this should be considered a bug on the tabulated-list end or not.


[-- Attachment #2: 0001-Fix-tabulated-list-widen-current-column-widening-wro.patch --]
[-- Type: text/x-patch, Size: 1806 bytes --]

From 4306519f28f5889c7fbf435c0d12b596f49a6a52 Mon Sep 17 00:00:00 2001
From: Thuna <thuna.cing@gmail.com>
Date: Tue, 18 Jan 2022 23:51:30 +0300
Subject: [PATCH] Fix tabulated-list-widen-current-column widening wrong column

* tabulated-list.el (tabulated-list-widen-current-column): Account for the
padding and the content width when calculating column width.
---
 lisp/emacs-lisp/tabulated-list.el | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/lisp/emacs-lisp/tabulated-list.el b/lisp/emacs-lisp/tabulated-list.el
index 4a9814b5da..32a046e0fb 100644
--- a/lisp/emacs-lisp/tabulated-list.el
+++ b/lisp/emacs-lisp/tabulated-list.el
@@ -731,6 +731,7 @@ tabulated-list-widen-current-column
 1."
   (interactive "p")
   (let ((start (current-column))
+        (entry (tabulated-list-get-entry))
         (nb-cols (length tabulated-list-format))
         (col-nb 0)
         (total-width 0)
@@ -741,9 +742,14 @@ tabulated-list-widen-current-column
       (if (> start
              (setq total-width
                    (+ total-width
-                      (setq col-width
-                            (cadr (aref tabulated-list-format
-                                        col-nb))))))
+                      (max (setq col-width
+                                 (cadr (aref tabulated-list-format
+                                             col-nb)))
+                           (string-width (aref entry col-nb)))
+                      (or (plist-get (nthcdr 3 (aref tabulated-list-format
+                                                     col-nb))
+                                     :pad-right)
+                          1))))
           (setq col-nb (1+ col-nb))
         (setq found t)
         (setf (cadr (aref tabulated-list-format col-nb))
-- 
2.25.1

bug#53375: [PATCH] Account for padding and content length in, tabulated-list-widen-current-column

[Lars Ingebrigtsen]

Thuna <thuna.cing@gmail.com> writes:

> This patch should solve the issue however it relies on
> `tabulated-list-get-entry' which means it fails to work if the
> `tabulated-list-entry' property is not set properly.  I am unsure if
> this should be considered a bug on the tabulated-list end or not.

That function is supposed to give reliable results, so if there are
modes it doesn't work in, that sounds like a bug.

Anyway, the patch looks good to me, so I've pushed it to Emacs 29.

This change was small enough to apply without assigning copyright to the
FSF, but for future patches you want to submit, it might make sense to
get the paperwork started now, so that subsequent patches can be applied
speedily. Would you be willing to sign such paperwork?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#53375: [PATCH] Account for padding and content length in, tabulated-list-widen-current-column

[Thuna]

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

  >> This patch should solve the issue however it relies on
 >> `tabulated-list-get-entry' which means it fails to work if the
 >> `tabulated-list-entry' property is not set properly.  I am unsure if
 >> this should be considered a bug on the tabulated-list end or not.
 >
 > That function is supposed to give reliable results, so if there are
 > modes it doesn't work in, that sounds like a bug.

In that case debbugs itself needs a fix.  It is possible that other
tabulated-list derived modes will fail in a similar way so maybe a
checkup is needed?

 > This change was small enough to apply without assigning copyright to the
 > FSF, but for future patches you want to submit, it might make sense to
 > get the paperwork started now, so that subsequent patches can be applied
 > speedily. Would you be willing to sign such paperwork?

I am willing to sign the paperwork although I know neither how nor what
it entails so some help would be appreciated.

On Thu, Jan 20, 2022 at 5:23 PM Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Thuna <thuna.cing@gmail.com> writes:
>
> > This patch should solve the issue however it relies on
> > `tabulated-list-get-entry' which means it fails to work if the
> > `tabulated-list-entry' property is not set properly.  I am unsure if
> > this should be considered a bug on the tabulated-list end or not.
>
> That function is supposed to give reliable results, so if there are
> modes it doesn't work in, that sounds like a bug.
>
> Anyway, the patch looks good to me, so I've pushed it to Emacs 29.
>
> This change was small enough to apply without assigning copyright to the
> FSF, but for future patches you want to submit, it might make sense to
> get the paperwork started now, so that subsequent patches can be applied
> speedily. Would you be willing to sign such paperwork?
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no
>

[-- Attachment #2: Type: text/html, Size: 2864 bytes --]

bug#53375: [PATCH] Account for padding and content length in, tabulated-list-widen-current-column

[Lars Ingebrigtsen]

Thuna <thuna.cing@gmail.com> writes:

>  > That function is supposed to give reliable results, so if there are
>  > modes it doesn't work in, that sounds like a bug.
>
> In that case debbugs itself needs a fix.  It is possible that other
> tabulated-list derived modes will fail in a similar way so maybe a
> checkup is needed?

Sounds like it.  

>  > This change was small enough to apply without assigning copyright to the
>  > FSF, but for future patches you want to submit, it might make sense to
>  > get the paperwork started now, so that subsequent patches can be applied
>  > speedily. Would you be willing to sign such paperwork?
>
> I am willing to sign the paperwork although I know neither how nor what
> it entails so some help would be appreciated.

Here's the form to get started, and here's the rationale behind it:

  https://www.gnu.org/licenses/why-assign.en.html
  

Please email the following information to assign@gnu.org, and we
will send you the assignment form for your past and future changes.

Please use your full legal name (in ASCII characters) as the subject
line of the message.
----------------------------------------------------------------------
REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES

[What is the name of the program or package you're contributing to?]
Emacs

[Did you copy any files or text written by someone else in these changes?
Even if that material is free software, we need to know about it.]

[Do you have an employer who might have a basis to claim to own
your changes?  Do you attend a school which might make such a claim?]

[For the copyright registration, what country are you a citizen of?]

[What year were you born?]

[Please write your email address here.]

[Please write your postal address here.]

[Which files have you changed so far, and which new files have you written
so far?]

bug#53375: [PATCH] Correct wrong comparison operation in previous patch

[Thuna]

It appears the previous patch I sent used the wrong comparison
operation (">" instead of ">=") and as a result widened the wrong
column when point was at the first character.  The following patch
resolves the issue.

<#part type="text/x-diff" filename="/home/thuna/emacs/0001-Fix-tabulated-list-widen-current-column-widening-wro.patch" disposition=inline>
<#/part>

bug#53375: [PATCH] Correct wrong comparison operation in previous patch

[Lars Ingebrigtsen]

Thuna <thuna.cing@gmail.com> writes:

> It appears the previous patch I sent used the wrong comparison
> operation (">" instead of ">=") and as a result widened the wrong
> column when point was at the first character.  The following patch
> resolves the issue.
>
> <#part type="text/x-diff"
> filename="/home/thuna/emacs/0001-Fix-tabulated-list-widen-current-column-widening-wro.patch"
> disposition=inline>
> <#/part>

Apparently the patch didn't make it here.  Can you resend it?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#53375: [PATCH] Correct wrong comparison operation in previous patch

[Thuna]

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

My bad, hopefully this goes through.

[-- Attachment #2: 0001-Fix-tabulated-list-widen-current-column-widening-wro.patch --]
[-- Type: text/x-patch, Size: 2086 bytes --]

From cd255476bf219e8162f56525e66c93fc5da3a3a9 Mon Sep 17 00:00:00 2001
From: Thuna <thuna.cing@gmail.com>
Date: Sun, 23 Jan 2022 17:07:33 +0300
Subject: [PATCH] Fix tabulated-list-widen-current-column widening wrong column

* tabulated-list.el (tabulated-list-widen-current-column): Use correct
comparison operation when checking if point is within the column
bounds.
Copyright-paperwork-exempt: yes
---
 lisp/emacs-lisp/tabulated-list.el | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/lisp/emacs-lisp/tabulated-list.el b/lisp/emacs-lisp/tabulated-list.el
index 32a046e0fb..a242ac1899 100644
--- a/lisp/emacs-lisp/tabulated-list.el
+++ b/lisp/emacs-lisp/tabulated-list.el
@@ -739,17 +739,17 @@ tabulated-list-widen-current-column
         col-width)
     (while (and (not found)
                 (< col-nb nb-cols))
-      (if (> start
-             (setq total-width
-                   (+ total-width
-                      (max (setq col-width
-                                 (cadr (aref tabulated-list-format
-                                             col-nb)))
-                           (string-width (aref entry col-nb)))
-                      (or (plist-get (nthcdr 3 (aref tabulated-list-format
-                                                     col-nb))
-                                     :pad-right)
-                          1))))
+      (if (>= start
+              (setq total-width
+                    (+ total-width
+                       (max (setq col-width
+                                  (cadr (aref tabulated-list-format
+                                              col-nb)))
+                            (string-width (aref entry col-nb)))
+                       (or (plist-get (nthcdr 3 (aref tabulated-list-format
+                                                      col-nb))
+                                      :pad-right)
+                           1))))
           (setq col-nb (1+ col-nb))
         (setq found t)
         (setf (cadr (aref tabulated-list-format col-nb))
-- 
2.25.1

bug#53375: [PATCH] Correct wrong comparison operation in previous patch

[Lars Ingebrigtsen]

Thuna <thuna.cing@gmail.com> writes:

> My bad, hopefully this goes through.

Yup; pushed to Emacs 29 now.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no