bug#26317: 26.0.50; Small incompatibility in ibuffer.el (ibuffer-compile-format)

bug#26317: 26.0.50; Small incompatibility in ibuffer.el (ibuffer-compile-format)

[John Mastro]

In commit 7e2a8b1, ibuffer.el was change to use `string-width' in a couple
places where it previously used `length'. The result is that custom columns
which can return nil, which worked in Emacs 25, will now cause an error.

To reproduce, run `emacs -Q', yank the following into *scratch*, and evaluate
it:

    (require 'ibuffer)

    (define-ibuffer-column my-column
      (:name "X")
      (and (equal (buffer-name) "*scratch*") "Y"))

    (setq ibuffer-formats '((mark modified read-only
                                  " " (name 18 18 :left :elide)
                                  " " (my-column 5 5 :left)
                                  " " filename-and-process)))

    (ibuffer)

It works in Emacs 25, but in Emacs 26 an error is signaled:

    Debugger entered--Lisp error: (wrong-type-argument stringp nil)
      string-width(nil)

I came across this because I use the `ibuffer-vc' package, which defines a
column (`vc-status') whose implementation returns nil for some buffers (by
default, non-file-visiting buffers and buffers visiting remote files).

I will follow up with a patch in a separate email (so I can include the bug
number in the commit message).

        John

bug#26317: 26.0.50; Small incompatibility in ibuffer.el (ibuffer-compile-format)

[John Mastro]

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

John Mastro <john.b.mastro@gmail.com> wrote:
> In commit 7e2a8b1, ibuffer.el was change to use `string-width' in a couple
> places where it previously used `length'. The result is that custom columns
> which can return nil, which worked in Emacs 25, will now cause an error.

I've attached a patch that fixes this. Rather than touching the calls to
`string-width' directly, it translates nil to the empty string. Either
way restores compatibility with Emacs 25, but translating nil to the
empty string seems preferable in that it saves later code from caring.

Even though it's a tiny change, I'm not a regular contributor or
particularly familiar with ibuffer's implementation, so any review is
appreciated. My copyright assignment is on file.

        John

[-- Attachment #2: 0001-Fix-a-small-incompatibility-in-ibuffer.patch --]
[-- Type: application/octet-stream, Size: 1038 bytes --]

From 81c03b3f37274ef2247903467ccbb7c8970ff976 Mon Sep 17 00:00:00 2001
From: John Mastro <john.b.mastro@gmail.com>
Date: Thu, 30 Mar 2017 16:01:41 -0700
Subject: [PATCH] Fix a small incompatibility in ibuffer

Translate nil values from column functions to the empty string, so that
subsequent calls to string-width don't signal an error (Bug#26317).
* lisp/ibuffer.el (ibuffer-compile-format): If a column function returns
nil, treat it like the empty string.
---
 lisp/ibuffer.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/ibuffer.el b/lisp/ibuffer.el
index 66916b2..c83c213 100644
--- a/lisp/ibuffer.el
+++ b/lisp/ibuffer.el
@@ -1689,7 +1689,7 @@ ibuffer-compile-format
 		    ;; generate a call to the column function.
 		    (ibuffer-aif (assq sym ibuffer-inline-columns)
 			(nth 1 it)
-		      `(,sym buffer mark)))
+		      `(or (,sym buffer mark) "")))
 		   ;; You're not expected to understand this.  Hell, I
 		   ;; don't even understand it, and I wrote it five
 		   ;; minutes ago.
-- 
2.10.2

bug#26317: 26.0.50; Small incompatibility in ibuffer.el (ibuffer-compile-format)

[Mark Oteiza]

John Mastro <john.b.mastro@gmail.com> writes:

> John Mastro <john.b.mastro@gmail.com> wrote:
>> In commit 7e2a8b1, ibuffer.el was change to use `string-width' in a couple
>> places where it previously used `length'. The result is that custom columns
>> which can return nil, which worked in Emacs 25, will now cause an error.
>
> I've attached a patch that fixes this. Rather than touching the calls to
> `string-width' directly, it translates nil to the empty string. Either
> way restores compatibility with Emacs 25, but translating nil to the
> empty string seems preferable in that it saves later code from caring.
>
> Even though it's a tiny change, I'm not a regular contributor or
> particularly familiar with ibuffer's implementation, so any review is
> appreciated. My copyright assignment is on file.

LGTM.  Sorry for breaking it, and thanks for the fix!

Applied as eeaa707.