Re: [ELPA] New package: marginalia

[Juri Linkov]

> What about adding the extended `symbol-class` function from Marginalia
> directly to help-fns.el? Note that the Marginalia function returns
> multiple characters "fv" if a symbol denotes both a function and a variable.
>
> Do you want to keep the `symbol-class` to be a single character under
> all circumstances?

I have tried to use marginalia--symbol-class for help--symbol-class,
and noticed a few problems:

1. It's hard to visually scan the column with types
   when the primary char (f, c, v, u) is not the first in the row.
   In the following patch I moved the primary char to the beginning.

2. Also it would help to see the letter of the primary char
   if secondary chars are not letters.  So I replaced "o" (obsolete)
   with a strikethrough symbol "-".

3. Are Unicode characters allowed?  Then could we add to the primary char
   e.g. COMBINING LONG STROKE OVERLAY (U+0336) to designate obsolete.

4. The remaining secondary letter is "l" for local, but I can't find
   a non-letter character to replace it.

5. Isn't using formatting such as "%-6s" not suitable in the backend?
   At least, I moved it out of help--symbol-class.  Ideally,
   the frontend should adjust column sizes to fit them to the longest value.

6. The format "%-6s " with 7 placeholders that is too wide for most symbols
   when a tiny fraction of symbols has 4 chars max, so reduced it to 4:

diff --git a/lisp/help-fns.el b/lisp/help-fns.el
index 133763add1..804ab43063 100644
--- a/lisp/help-fns.el
+++ b/lisp/help-fns.el
@@ -130,23 +130,25 @@ help--symbol-class
   "Return symbol class characters for symbol S."
   (when (stringp s)
     (setq s (intern-soft s)))
-  (cond ((commandp s)
-         "c")                           ; command
-        ((eq (car-safe (symbol-function s)) 'macro)
-         "m")                           ; macro
-        ((fboundp s)
-         "f")                           ; function
-        ((custom-variable-p s)
-         "u")                           ; user option
-        ((boundp s)
-         "v")                           ; variable
-        ((facep s)
-         "a")                           ; fAce
-        ((and (fboundp 'cl-find-class)
-              (cl-find-class s))
-         "t")                           ; CL type
-        (" ")                           ; something else
-        ))
+  (concat
+   (when (fboundp s)
+     (concat
+      (cond
+       ((commandp s) "c")
+       ((eq (car-safe (symbol-function s)) 'macro) "m")
+       (t "f"))
+      (and (let ((flist (indirect-function s)))
+             (advice--p (if (eq 'macro (car-safe flist)) (cdr flist) flist)))
+           "!")
+      (and (get s 'byte-obsolete-info) "-")))
+   (when (boundp s)
+     (concat
+      (if (custom-variable-p s) "u" "v")
+      (and (local-variable-if-set-p s) "l")
+      (and (ignore-errors (not (equal (symbol-value s) (default-value s)))) 
"*")
+      (and (get s 'byte-obsolete-variable) "-")))
+   (and (facep s) "a")
+   (and (fboundp 'cl-find-class) (cl-find-class s) "t")))
 
 (defun help--symbol-completion-table-affixation (completions)
   (mapcar (lambda (c)
@@ -154,7 +156,7 @@ help--symbol-completion-table-affixation
                    (doc (condition-case nil (documentation s) (error nil)))
                    (doc (and doc (substring doc 0 (string-match "\n" doc)))))
               (list c (propertize
-                       (concat (help--symbol-class s) " ") ; prefix separator
+                       (format "%-4s " (help--symbol-class s))
                        'face 'completions-annotations)
                     (if doc (propertize (format " -- %s" doc)
                                         'face 'completions-annotations)