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

[Basil L. Contovounesios]

"Peter Feigl" <peter.feigl@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