GNU bug report logs - #44711
[PATCH] Add movement commands `tabulated-list-next-column' and `tabulated-list-previous-column'

Previous Next

Package: emacs;

Reported by: "Peter Feigl" <peter.feigl <at> nexoid.at>

Date: Tue, 17 Nov 2020 17:53:01 UTC

Severity: normal

Tags: patch

Fixed in version 28.1

Done: Lars Ingebrigtsen <larsi <at> gnus.org>

Bug is archived. No further changes may be made.

Full log


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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: "Peter Feigl" <peter.feigl <at> nexoid.at>
Cc: 44711 <at> debbugs.gnu.org
Subject: Re: bug#44711: [PATCH] Add movement commands
 `tabulated-list-next-column' and `tabulated-list-previous-column'
Date: Tue, 17 Nov 2020 18:35:52 +0000
"Peter Feigl" <peter.feigl <at> nexoid.at> writes:

> The attached patch adds two movement commands,

Thanks, just some minor comments from me.

> +(defun tabulated-list-next-column (&optional arg)
> +  "Go to the start of the next column after point on the current line.
> +If ARG is provided, move that many columns."
> +  (interactive "p")
> +  (dotimes (c (or arg 1))
               ^
The names of unused lexical variables should start with (or consist only
of) an underscore, e.g. '_' or '_c'.  (The byte-compiler should
otherwise complain.)

> +    (let ((next (or (next-single-property-change (point) 'tabulated-list-column-name) (point-max))))

This line is a bit long; suggest breaking the function call across two
lines.  More importantly, why scan all the way to the end of the buffer
if we're only interested in the current line?

> +      (unless (>= next (line-end-position))
> +        (goto-char next)))))

Given my suggestion to limit the search to the current line, this could
be modified along the following lines:

  (dotimes (_ (or arg 1))
    (let* ((eol (line-end-position))
           (next (next-single-property-change
                  (point) 'tabulated-list-column-name nil (1+ eol))))
      (when (< next eol)
        (goto-char next))))

But there's a subtle issue here: some tabulated lists
(e.g. list-buffers) can have an empty final column (e.g. with
non-file-visiting buffers such as *scratch*), but the (< next eol) guard
will not allow point to reach this final empty column, because that
position is at the end of the line.

So I suggest either augmenting the guard to check whether the current
(last) column is empty, or unconditionally allowing point to reach the
end of the line, even if point was already originally in the final
column.

> +(defun tabulated-list-previous-column (&optional arg)

Ditto.

-- 
Basil




This bug report was last modified 3 years and 356 days ago.

Previous Next


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