bug#24543: 25.1; describe-symbol doesn't show face with same name as a variable

bug#24543: 25.1; describe-symbol doesn't show face with same name as a variable

[Alex]

Recipe:

emacs -Q
C-h o font-lock-comment-face RET

Only the variable of this name is shown, not the face (which can be
viewed with describe-face, of course).

It appears that when the face only shares a name with a function, then
both the function and face are shown. Only a variable shadows the face
in describe-symbol.

bug#24543: 25.1; describe-symbol doesn't show face with same name as a variable

[Alex]

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

Alex <agrambot@gmail.com> writes:

> Recipe:
>
> emacs -Q
> C-h o font-lock-comment-face RET
>
> Only the variable of this name is shown, not the face (which can be
> viewed with describe-face, of course).
>
> It appears that when the face only shares a name with a function, then
> both the function and face are shown. Only a variable shadows the face
> in describe-symbol.

This is because describe-symbol isn't returning (buffer-string) like
describe-{function, variable} are.

I've attached a patch fixing this problem.

Additionally, I have attached another patch that re-orders the placement
of variables/faces in describe-symbol, to put more emphasis on the
variable entry rather than the face.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fix --]
[-- Type: text/x-diff, Size: 845 bytes --]

From 9a3196624d3a24beb3a0625e8a2a74751368c112 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Fri, 16 Jun 2017 15:37:47 -0600
Subject: [PATCH 1/2] Properly show faces in describe-symbol (Bug#24543)

* lisp/faces.el (describe-face): Return (buffer-string).
---
 lisp/faces.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/faces.el b/lisp/faces.el
index 9a8a1344ca..dabb7c3e7e 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -1447,7 +1447,7 @@ describe-face
 	(setq face (list face)))
     (with-help-window (help-buffer)
       (with-current-buffer standard-output
-	(dolist (f face)
+	(dolist (f face (buffer-string))
 	  (if (stringp f) (setq f (intern f)))
 	  ;; We may get called for anonymous faces (i.e., faces
 	  ;; expressed using prop-value plists).  Those can't be
-- 
2.11.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: reorder --]
[-- Type: text/x-diff, Size: 992 bytes --]

From 9ec7945fd45af4999a315f2ceba9bbc61cbf0710 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Fri, 16 Jun 2017 15:49:13 -0600
Subject: [PATCH 2/2] * lisp/help-mode.el (describe-symbol-backends): Reorder
 entries

---
 lisp/help-mode.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/help-mode.el b/lisp/help-mode.el
index 3fb793e7aa..24dfb9120b 100644
--- a/lisp/help-mode.el
+++ b/lisp/help-mode.el
@@ -393,12 +393,12 @@ help-buffer
 
 (defvar describe-symbol-backends
   `((nil ,#'fboundp ,(lambda (s _b _f) (describe-function s)))
-    ("face" ,#'facep ,(lambda (s _b _f) (describe-face s)))
     (nil
      ,(lambda (symbol)
         (or (and (boundp symbol) (not (keywordp symbol)))
             (get symbol 'variable-documentation)))
-     ,#'describe-variable)))
+     ,#'describe-variable)
+    ("face" ,#'facep ,(lambda (s _b _f) (describe-face s)))))
 
 ;;;###autoload
 (defun help-make-xrefs (&optional buffer)
-- 
2.11.0

bug#24543: 25.1; describe-symbol doesn't show face with same name as a variable

[Alex]

Alex <agrambot@gmail.com> writes:

> Alex <agrambot@gmail.com> writes:
>
>> Recipe:
>>
>> emacs -Q
>> C-h o font-lock-comment-face RET
>>
>> Only the variable of this name is shown, not the face (which can be
>> viewed with describe-face, of course).
>>
>> It appears that when the face only shares a name with a function, then
>> both the function and face are shown. Only a variable shadows the face
>> in describe-symbol.
>
> This is because describe-symbol isn't returning (buffer-string) like
> describe-{function, variable} are.
>
> I've attached a patch fixing this problem.
>
> Additionally, I have attached another patch that re-orders the placement
> of variables/faces in describe-symbol, to put more emphasis on the
> variable entry rather than the face.

Ping. Eli?

bug#24543: 25.1; describe-symbol doesn't show face with same name as a variable

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>
> Date: Tue, 25 Jul 2017 22:28:05 -0600
> 
> Ping. Eli?

Sorry, I hoped someone more knowledgeable about this stiff will beat
me to it.

I will review this in a couple of days.

bug#24543: 25.1; describe-symbol doesn't show face with same name as a variable

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Date: Fri, 16 Jun 2017 15:54:45 -0600
> 
> > emacs -Q
> > C-h o font-lock-comment-face RET
> >
> > Only the variable of this name is shown, not the face (which can be
> > viewed with describe-face, of course).
> >
> > It appears that when the face only shares a name with a function, then
> > both the function and face are shown. Only a variable shadows the face
> > in describe-symbol.
> 
> This is because describe-symbol isn't returning (buffer-string) like
> describe-{function, variable} are.
> 
> I've attached a patch fixing this problem.

Thanks.  This LGTM, but doesn't the fact that describe-face's return
value will change after this constitute an incompatible change?  And
if so, could it break some of the users of this function out there?

bug#24543: 25.1; describe-symbol doesn't show face with same name as a variable

[Alex]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Date: Fri, 16 Jun 2017 15:54:45 -0600
>> 
>> > emacs -Q
>> > C-h o font-lock-comment-face RET
>> >
>> > Only the variable of this name is shown, not the face (which can be
>> > viewed with describe-face, of course).
>> >
>> > It appears that when the face only shares a name with a function, then
>> > both the function and face are shown. Only a variable shadows the face
>> > in describe-symbol.
>> 
>> This is because describe-symbol isn't returning (buffer-string) like
>> describe-{function, variable} are.
>> 
>> I've attached a patch fixing this problem.
>
> Thanks.  This LGTM, but doesn't the fact that describe-face's return
> value will change after this constitute an incompatible change?  And

I suppose so; would it be expected to be documented in NEWS?

> if so, could it break some of the users of this function out there?

Potentially, but I don't see that being an issue in this case. The
return value isn't documented, so users shouldn't be (and likely aren't)
relying on describe-face always returning nil. It makes more sense to
treat describe-face like describe-{function, variable}.

Though I'm not sure why those two procedures return (buffer-string) in
the first place. In any case, I figure it's better to leave those and
describe-symbol alone and just make describe-face behave similarly.

bug#24543: 25.1; describe-symbol doesn't show face with same name as a variable

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: 24543@debbugs.gnu.org
> Date: Fri, 28 Jul 2017 14:45:24 -0600
> 
> > if so, could it break some of the users of this function out there?
> 
> Potentially, but I don't see that being an issue in this case. The
> return value isn't documented, so users shouldn't be (and likely aren't)
> relying on describe-face always returning nil. It makes more sense to
> treat describe-face like describe-{function, variable}.

OK, I pushed the changes.

Thanks.