bug#21025: tabulated-list.el: It should preserve header line set by derived modes. (Encl. a suggested PATCH)

bug#21025: tabulated-list.el: It should preserve header line set by derived modes. (Encl. a suggested PATCH)

[Vaidheeswaran C]

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


tabulated-list.el: It should preserve header line set by derived
modes.  Also some fontification improvements.

1. Load the attached font-family.el.

2. M-x list-font-families.

3. Note that there is no header line when I explicitly request that I
   need a header line @ lines 25, 26 in font-family.el

      (setq tabulated-list-use-header-line nil)
      (setq header-line-format "Font families")

----------------------------------------------------------------

4. Apply the attached patch.

5. See that the header line is preserved (and stays put even when the
   columns are sorted and the buffer is refreshed).

6. Admire the revamped look of header columns.  (See the attached
   screenshot)

----------------------------------------------------------------

edictc.el -- my all new DICT client for Emacs -- uses header lines
when in tabulated list mode.  (See the attached screenshot.)  Btw, the
red strip is coming from whitespace mode.

[-- Attachment #2: font-family.el --]
[-- Type: text/x-emacs-lisp, Size: 2570 bytes --]

(require 'tabulated-list)

(defvar-local font-family-menu-test-string nil
  "Test string to display in the menu.")

(defvar font-family-menu-mode-map
  (let ((map (make-sparse-keymap))
        (menu-map (make-sparse-keymap "Font Family")))
    (set-keymap-parent map tabulated-list-mode-map)
    (define-key map "\C-m" 'font-family-menu-set-frame-font)
    (define-key map "x" 'font-family-menu-set-test-string)
    map)
  "Local keymap for `font-family-menu-mode' buffers.")

(define-derived-mode font-family-menu-mode tabulated-list-mode "Font Family Menu"
  "Inspect font families and pick a frame font.
Display the name of the font family in that family.  Use
`font-family-set-test-string' to set a test string.

 \\<font-family-menu-mode-map> \\{font-family-menu-mode-map}"
  (setq tabulated-list-format
        `[("Font Family" 30 t)
          ("Text" 30 nil)])
  (setq tabulated-list-padding 2)
  (setq tabulated-list-sort-key (cons "Font Family" nil))
  (setq tabulated-list-use-header-line nil)
  (setq header-line-format "Font families")
  (add-hook 'tabulated-list-revert-hook 'font-family-menu--refresh nil t)
  (font-family-menu--refresh)
  (tabulated-list-init-header)
  (tabulated-list-print))

(defun list-font-families ()
  "Display the name of the font family using it's own face.
Use `font-family-menu-set-test-string' to display a test string
instead."
  (interactive)
  (let ((buf (get-buffer-create "*Font Families*")))
    (with-current-buffer buf
      (font-family-menu-mode))
    (switch-to-buffer buf)))

(defun font-family-menu--refresh ()
  "Re-populate `tabulated-list-entries'."
  (let ((f (delete-dups (font-family-list))))
    (setq tabulated-list-entries
	  (mapcar
	   (lambda (f)
	     (let ((s (or font-family-menu-test-string f)))
	       (list f (vector
			(cons f `(font-family ,f action font-family-menu-set-frame-font))
			(propertize s 'face (list :family f))))))
	   f))))

(defun font-family-menu-set-frame-font ()
  "Set the frame font to current one."
  (interactive)
  (when (derived-mode-p 'font-family-menu-mode)
      (let ((f (tabulated-list-get-id)))
	(when (and f (yes-or-no-p (concat "Set frame font to " f)))
	  (set-frame-font f nil t)))))

(defun font-family-menu-set-test-string (s)
  "Set the test string."
  (interactive "sDisplay string: ")
  (when (derived-mode-p 'font-family-menu-mode)
    (if (and (stringp s) (not (string= s "")))
	(setq font-family-menu-test-string s)
      (setq font-family-menu-test-string nil))
        (font-family-menu--refresh)
	(tabulated-list-print)))

(provide 'font-family)

[-- Attachment #3: tabulated-list.el.diff --]
[-- Type: text/x-patch, Size: 1085 bytes --]

diff --git a/lisp/emacs-lisp/tabulated-list.el b/lisp/emacs-lisp/tabulated-list.el
index 9119c3a..34b46dc 100644
--- a/lisp/emacs-lisp/tabulated-list.el
+++ b/lisp/emacs-lisp/tabulated-list.el
@@ -241,7 +241,6 @@ Populated by `tabulated-list-init-header'.")
     (setq cols (apply 'concat (nreverse cols)))
     (if tabulated-list-use-header-line
 	(setq header-line-format cols)
-      (setq header-line-format nil)
       (setq-local tabulated-list--header-string cols))))
 
 (defun tabulated-list-print-fake-header ()
@@ -255,7 +254,8 @@ Do nothing if `tabulated-list--header-string' is nil."
           (move-overlay tabulated-list--header-overlay (point-min) (point))
         (setq-local tabulated-list--header-overlay
                     (make-overlay (point-min) (point))))
-      (overlay-put tabulated-list--header-overlay 'face 'underline))))
+      (overlay-put tabulated-list--header-overlay
+                   'face '(:overline t :underline t :weight bold)))))
 
 (defun tabulated-list-revert (&rest ignored)
   "The `revert-buffer-function' for `tabulated-list-mode'.

[-- Attachment #4: tabulated-list-with-header-line-(after the fix).png --]
[-- Type: image/png, Size: 94089 bytes --]

[-- Attachment #5: a-preview-of-edictc.el-with-header-xref-buttons-and-header-line-in-tabulated-list-mode.png --]
[-- Type: image/png, Size: 127861 bytes --]

bug#21025: tabulated-list.el: It should preserve header line set by derived modes. (Encl. a suggested PATCH)

[Artur Malabarba]

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

> 3. Note that there is no header line when I explicitly request that I
>    need a header line @ lines 25, 26 in font-family.el
>
>       (setq tabulated-list-use-header-line nil)
>       (setq header-line-format "Font families")

When you set that to nil you are explicitly asking tabulated-list to erase
the header line. Just leave it as t instead.
As long as you don't call init-header, the header will be left as you set
it.

> ----------------------------------------------------------------
>
> 4. Apply the attached patch.
>
> 5. See that the header line is preserved (and stays put even when the
>    columns are sorted and the buffer is refreshed).

Although your patch is technically unnecessary (what you want to do is
already possible) it might be good to apply anyway.
The line of code that you remove in your patch might break stuff, but if it
doesn't then it should be removed. But I'm not the right person to say
that.

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

bug#21025: tabulated-list.el: It should preserve header line set by derived modes. (Encl. a suggested PATCH)

[Vaidheeswaran C]

On Monday 13 July 2015 04:46 AM, Artur Malabarba wrote:

> Although your patch is technically unnecessary (what you want to do is
> already possible)

Could you please share a recipe whereby I can get *both*

    a) the table header of the tabulated list.
    b) header line of the buffer.

The screenshots I attached shows what I hope to accomplish.

bug#21025: tabulated-list.el: It should preserve header line set by derived modes. (Encl. a suggested PATCH)

[Artur Malabarba]

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

> Could you please share a recipe whereby I can get *both*
>
>     a) the table header of the tabulated list.
>     b) header line of the buffer.
>
> The screenshots I attached shows what I hope to accomplish.

Sorry, I was under the impression that you only wanted to have a custom
header line. Indeed, to achieve both of those things, your patch is
necessary.

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

bug#21025: tabulated-list.el: It should preserve header line set by derived modes. (Encl. a suggested PATCH)

[Lars Ingebrigtsen]

Vaidheeswaran C <vaidheeswaran.chinnaraju@gmail.com> writes:

> 3. Note that there is no header line when I explicitly request that I
>    need a header line @ lines 25, 26 in font-family.el
>
>       (setq tabulated-list-use-header-line nil)
>       (setq header-line-format "Font families")

[...]

>      (if tabulated-list-use-header-line
>  	(setq header-line-format cols)
> -      (setq header-line-format nil)

I'm not very familiar with tabulated lists, but this seems like it's
fixing the problem you're seeing...

> -      (overlay-put tabulated-list--header-overlay 'face 'underline))))
> +      (overlay-put tabulated-list--header-overlay
> +                   'face '(:overline t :underline t :weight bold)))))

This might be more controversial, though.  I think it looks nice, but
some people don't like so many lines.  Perhaps it should be a defface,
so that people can change this themselves?  The default could be what
you're describing, though.

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

bug#21025: tabulated-list.el: It should preserve header line set by derived modes. (Encl. a suggested PATCH)

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> -      (overlay-put tabulated-list--header-overlay 'face 'underline))))
>> +      (overlay-put tabulated-list--header-overlay
>> +                   'face '(:overline t :underline t :weight bold)))))
>
> This might be more controversial, though.  I think it looks nice, but
> some people don't like so many lines.  Perhaps it should be a defface,
> so that people can change this themselves?  The default could be what
> you're describing, though.

I've now done this change and otherwise applied your patch.

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

bug#21025: tabulated-list.el: It should preserve header line set by derived modes. (Encl. a suggested PATCH)

[Drew Adams]

> >> -      (overlay-put tabulated-list--header-overlay 'face 'underline))))
> >> +      (overlay-put tabulated-list--header-overlay
> >> +                   'face '(:overline t :underline t :weight bold)))))
> >
> > This might be more controversial, though.  I think it looks nice, but
> > some people don't like so many lines.  Perhaps it should be a defface,
> > so that people can change this themselves?  The default could be what
> > you're describing, though.
> 
> I've now done this change and otherwise applied your patch.

This is a bad way to do things - makes it difficult
for users do customize the appearance.  Emacs is "the
extensible, customizable, self-documenting, real-time
display editor".

The right way to take care of what is essentially a
user's preference for column-header appearance is not
to change the hard-coded appearance but to define a
face for this in `tabulated-list.el'.  Then ALL users
can get the appearance they want just by customizing
that face.

The face can have whatever default appearance you
want: underline, underline and overline - whatever.
But let users customize a face; do not hard-code one.

bug#21025: tabulated-list.el: It should preserve header line set by derived modes. (Encl. a suggested PATCH)

[Lars Ingebrigtsen]

Drew Adams <drew.adams@oracle.com> writes:

>> >> -      (overlay-put tabulated-list--header-overlay 'face 'underline))))
>> >> +      (overlay-put tabulated-list--header-overlay
>> >> +                   'face '(:overline t :underline t :weight bold)))))
>> >
>> > This might be more controversial, though.  I think it looks nice, but
>> > some people don't like so many lines.  Perhaps it should be a defface,
>> > so that people can change this themselves?  The default could be what
>> > you're describing, though.
>> 
>> I've now done this change and otherwise applied your patch.
>
> This is a bad way to do things - makes it difficult
> for users do customize the appearance.  Emacs is "the
> extensible, customizable, self-documenting, real-time
> display editor".
>
> The right way to take care of what is essentially a
> user's preference for column-header appearance is not
> to change the hard-coded appearance but to define a
> face for this in `tabulated-list.el'.  Then ALL users
> can get the appearance they want just by customizing
> that face.

That's what I did, and that's what I said I did.

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

bug#21025: tabulated-list.el: It should preserve header line set by derived modes. (Encl. a suggested PATCH)

[Drew Adams]

> That's what I did, and that's what I said I did.

Sorry; my bad - didn't read well.  I thought the
bit of patch quoted was applied as is.  Thx for
DTRT.