Re: Ideas to improve the output of C-h m?

["Clément Pit-Claudel" <cpitclaudel@gmail.com>]

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

Hi Stefan,

On 01/05/2020 19.02, Stefan Kangas wrote:
> Thank you for working on this.  I like the look of the screenshot, but
> you didn't include a patch.  If you could provide one, I may be able
> to give you more useful feedback on the basis of testing it.

Thanks a lot for having a look.  I have attached the code; this version is extracted from biblio.el, for which it was originally written.  After loading it you should be able to use M-x help-with-major-mode and M-: (help-with-keymap occur-mode-map), for example.

There are shortcomings in the current implementation (it iterates over nested keymaps to flatten them, and that part is likely incomplete), including the fact that it displays <remap …> rather than actual keybindings when a keymap includes remapping.  All of these would have to be ironed out as part of turning this into a patch extending the existing facilities, of course.

> One related idea comes to mind: (…)> It would be useful, I think, if the mode author was able to customize
> the listing of commands while also "automatically" using the best
> standard for displaying them (with the added faces, indentation, etc.
> as you suggest) rather than having to write a doc string manually to
> achieve it.  For example, maybe we could come up with a way to group
> commands by categories and then have them displayed under proper
> headlines in key binding listings.

Yes, that would be great.  If the current discussion does get somewhere, it would be a natural next step to work on, I think.

> Footnotes:
> 1. As an aside, I have started converting 'substite-command-keys' from
> C to Lisp some time ago, but I got stuck somewhere and didn't complete
> it.  I hope I can find the energy and time to finish that job.

I think they may be some overlap with the code I posted, as I had to write much of the iteration over keymaps from scratch.  Maybe both implementations can benefit from each other and we can get something done together.  Or maybe I can be the first client of your new code :)

Cheers,
Clément.


[-- Attachment #2: help-with-major-mode.el --]
[-- Type: text/x-emacs-lisp, Size: 3852 bytes --]

(require 'seq)
(require 'pcase)

(defsubst biblio--as-list (x)
  "Make X a list, if it isn't."
  (if (consp x) x (list x)))

(defun biblio--map-keymap (func map)
  "Call `map-keymap' on FUNC and MAP, and collect the results."
  (let ((out))
    (map-keymap (lambda (&rest args) (push (apply func args) out)) map)
    (nreverse out)))

(defun biblio--flatten-map (keymap &optional prefix)
  "Flatten KEYMAP, prefixing its keys with PREFIX.
This should really be in Emacs core (in Elisp), instead of being
implemented in C (at least for sparse keymaps).  Don't run this on
non-sparse keymaps."
  (nreverse
   (cond
    ((keymapp keymap)
     (seq-map (lambda (key-value)
                "Add PREFIX to key in KEY-VALUE."
                (cons (append prefix (biblio--as-list (car key-value)))
                      (cdr key-value)))
              (delq nil
                    (apply
                     #'seq-concatenate
                     'list (biblio--map-keymap
                            (lambda (k v)
                              "Return a list of bindings in V, prefixed by K."
                              (biblio--flatten-map v (biblio--as-list k)))
                            keymap)))))
    ;; FIXME This breaks if keymap is a symbol whose function cell is a keymap
    ((symbolp keymap)
     (list (cons prefix keymap))))))

(defun biblio--group-alist (alist)
  "Return a copy of ALIST whose keys are lists of keys, grouped by value.
That is, if two key map to `eq' values, they are grouped."
  (let ((map (make-hash-table :test 'eq))
        (new-alist nil))
    (pcase-dolist (`(,key . ,value) alist)
      (puthash value (cons key (gethash value map)) map))
    (pcase-dolist (`(,_ . ,value) alist)
      (let ((keys (gethash value map)))
        (when keys
          (push (cons (nreverse keys) value) new-alist)
          (puthash value nil map))))
    (nreverse new-alist)))

(defun biblio--quote (str)
  "Quote STR and call `substitute-command-keys' on it."
  (if str (substitute-command-keys (concat "`" str "'")) ""))

(defun biblio--quote-keys (keys)
  "Quote and concatenate keybindings in KEYS."
  (mapconcat (lambda (keyseq)
               (let ((key (ignore-errors (help-key-description keyseq nil))))
                 (if (and nil key (string-match-p " " key))
                     (biblio--quote key)
                   key)))
             keys ", "))

(defun biblio--brief-docs (command)
  "Return first line of documentation of COMMAND."
  (let ((docs (or (ignore-errors (documentation command t)) "")))
    (string-match "\\(.*\\)$" docs)
    (match-string-no-properties 1 docs)))

(defun biblio--help-with-major-mode-1 (keyseqs-command)
  "Print help on KEYSEQS-COMMAND to standard output."
  ;; (biblio-with-fontification 'font-lock-function-name-face
  (insert (biblio--quote-keys (car keyseqs-command)) " ")
  (insert (propertize "\t" 'display '(space :align-to 10)))
  ;; (insert "(")
  (insert-text-button (format "%S" (cdr keyseqs-command)))
  ;; (insert ")")
  (insert "\n")
  (insert (propertize (format "  %s\n" (biblio--brief-docs (cdr keyseqs-command)))
                      'face '(font-lock-comment-face (:height 0.95))))
  (insert (propertize "\n" 'face '(:height 0.3))))

(defun help-with-keymap (map &optional buf title)
  "Display help for keymap MAP in buffer BUF, with a given TITLE."
  (setq buf (or buf "*Keymap help*"))
  (with-help-window buf
    (when title (princ title))
    (let ((bindings (nreverse (biblio--group-alist (biblio--flatten-map map)))))
      (with-current-buffer buf
        (seq-do #'biblio--help-with-major-mode-1 bindings)))))

(defun help-with-major-mode ()
  "Display help for current major mode."
  (interactive)
  (help-with-keymap
   (current-local-map)
   (format "*%S help*" major-mode)
   (format "Help with %s\n\n" (biblio--quote (symbol-name major-mode)))))