bug#27890: 26.0.50; describe-font does not use Help buffer history

bug#27890: 26.0.50; describe-font does not use Help buffer history

[Alex]

describe-font does not add to the *Help* buffer's history. The [Back]
button does appear when there is a non-empty history, but going back and
then forward does not restore the font description.

bug#27890: 26.0.50; describe-font does not use Help buffer history

[Alex]

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

Alex <agrambot@gmail.com> writes:

> describe-font does not add to the *Help* buffer's history. The [Back]
> button does appear when there is a non-empty history, but going back and
> then forward does not restore the font description.

Here's a patch for it.


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

From 658823b3a1adec25283d28e59710aa160afd7908 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Sat, 5 Aug 2017 16:09:54 -0600
Subject: [PATCH] Use help-mode xrefs in describe-font

* lisp/international/mule-diag.el (describe-font): Use help-setup-xref
(Bug#27890).
---
 lisp/international/mule-diag.el | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lisp/international/mule-diag.el b/lisp/international/mule-diag.el
index c274621f77..655a5ca6d4 100644
--- a/lisp/international/mule-diag.el
+++ b/lisp/international/mule-diag.el
@@ -838,7 +838,8 @@ describe-font
   (interactive "sFont name (default current choice for ASCII chars): ")
   (or (and window-system (fboundp 'fontset-list))
       (error "No fonts being used"))
-  (let (font-info)
+  (let ((xref-item (list #'describe-font fontname))
+        font-info)
     (if (or (not fontname) (= (length fontname) 0))
 	(setq fontname (face-attribute 'default :font)))
     (setq font-info (font-info fontname))
@@ -850,6 +851,7 @@ describe-font
 	    ;; this problem.
 	    (message "No information about \"%s\"" (font-xlfd-name fontname))
 	  (message "No matching font found"))
+      (help-setup-xref xref-item (called-interactively-p 'interactive))
       (with-output-to-temp-buffer "*Help*"
 	(describe-font-internal font-info)))))
 
-- 
2.13.2

bug#27890: 26.0.50; describe-font does not use Help buffer history

[npostavs]

tags 27890 + patch
quit

Alex <agrambot@gmail.com> writes:

> Alex <agrambot@gmail.com> writes:
>
>> describe-font does not add to the *Help* buffer's history. The [Back]
>> button does appear when there is a non-empty history, but going back and
>> then forward does not restore the font description.
>
> Here's a patch for it.
>
> +  (let ((xref-item (list #'describe-font fontname))
[...]
> +      (help-setup-xref xref-item (called-interactively-p 'interactive))

Looks fine, personally I wouldn't bother introducing that local
variable.

bug#27890: 26.0.50; describe-font does not use Help buffer history

[Alex]

npostavs@users.sourceforge.net writes:

> tags 27890 + patch
> quit
>
> Alex <agrambot@gmail.com> writes:
>
>> Alex <agrambot@gmail.com> writes:
>>
>>> describe-font does not add to the *Help* buffer's history. The [Back]
>>> button does appear when there is a non-empty history, but going back and
>>> then forward does not restore the font description.
>>
>> Here's a patch for it.
>>
>> +  (let ((xref-item (list #'describe-font fontname))
> [...]
>> +      (help-setup-xref xref-item (called-interactively-p 'interactive))
>
> Looks fine, personally I wouldn't bother introducing that local
> variable.

I introduced that variable because the procedure conditionally sets
fontname if it's an empty sequence. As it stands, the procedure would
error without capturing that variable ahead of time due to `length' being
called on a font object.

I figured this was the simplest way around that problem. Do you have an
easier way in mind? I could perhaps change the setq line to instead set
fontname to (face-attribute 'default :family), which would allow for
removing the local variable safely.

bug#27890: 26.0.50; describe-font does not use Help buffer history

[Alex]

Alex <agrambot@gmail.com> writes:

> npostavs@users.sourceforge.net writes:
>
>> tags 27890 + patch
>> quit
>>
>> Alex <agrambot@gmail.com> writes:
>>
>>> Here's a patch for it.
>>>
>>> +  (let ((xref-item (list #'describe-font fontname))
>> [...]
>>> +      (help-setup-xref xref-item (called-interactively-p 'interactive))
>>
>> Looks fine, personally I wouldn't bother introducing that local
>> variable.
>
> I introduced that variable because the procedure conditionally sets
> fontname if it's an empty sequence. As it stands, the procedure would
> error without capturing that variable ahead of time due to `length' being
> called on a font object.
>
> I figured this was the simplest way around that problem. Do you have an
> easier way in mind? I could perhaps change the setq line to instead set
> fontname to (face-attribute 'default :family), which would allow for
> removing the local variable safely.

I made a patch that does this, but it doesn't behave exactly the same.
(font-info (face-attribute 'default :family)) for me returns a font with
size/height 16/19, but (font-info (face-attribute 'default :font))
returns a font with size/height 15/18. I've tried this with multiple
fonts, and it happens with each one.

This seems like a bug.

bug#27890: 26.0.50; describe-font does not use Help buffer history

[npostavs]

Alex <agrambot@gmail.com> writes:

> npostavs@users.sourceforge.net writes:
>
>> Looks fine, personally I wouldn't bother introducing that local
>> variable.
>
> I introduced that variable because the procedure conditionally sets
> fontname if it's an empty sequence. As it stands, the procedure would
> error without capturing that variable ahead of time due to `length' being
> called on a font object.

Oops, right.  I skimmed too quickly because your patch looked so simple.

> I figured this was the simplest way around that problem. Do you have an
> easier way in mind? I could perhaps change the setq line to instead set
> fontname to (face-attribute 'default :family), which would allow for
> removing the local variable safely.

No, let's not get bogged down in micro-optimization and just go with
what you had originally.

bug#27890: 26.0.50; describe-font does not use Help buffer history

[Alex]

npostavs@users.sourceforge.net writes:

> No, let's not get bogged down in micro-optimization and just go with
> what you had originally.

Sounds good. Would you please commit it for me at an appropriate time?

Thanks.

bug#27890: 26.0.50; describe-font does not use Help buffer history

[npostavs]

tags 27890 fixed
close 27890 26.1
quit

Alex <agrambot@gmail.com> writes:

> npostavs@users.sourceforge.net writes:
>
>> No, let's not get bogged down in micro-optimization and just go with
>> what you had originally.
>
> Sounds good. Would you please commit it for me at an appropriate time?

Pushed to master.

[1: 9723782161]: 2017-08-08 19:57:24 -0400
  Use help-mode xrefs in describe-font
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=9723782161f9b1ea6c3ce883acdc1090172b01c3