bug#22957: C-u C-x = prints garbage for a text property with value '(string . ?")

bug#22957: C-u C-x = prints garbage for a text property with value '(string . ?")

[Alan Mackenzie]

Hello, Emacs.

In the master branch:
  emacs -Q
.
In the *scratch* buffer, with point at any character, do:

  M-: (put-text-property (point) (1+ (point)) 'asdf '(string . ?\"))

.  Without moving point,

  C-u C-x =

.  There appears in the *Help* buffer the following:

  There are text properties here:
    asdf                 string (widget)Top
    face                 font-lock-comment-face
    fontified            t

.  The entry for "asdf" is garbage.  It should show the actual value of
the text property just set, not "string (widget)Top".  The same thing
happens in Emacs 24.5 (and presumably the emacs-25 branch).

#########################################################################

Initial investigation:

The symbol 'string has a 'widget-type property.  Without validating the
structure (or lack thereof) of the assumed widget value,
`describe-property-list' attempts to enter the "widget browser" rather
than printing the full value of the 'asdf text property.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#22957: C-u C-x = prints garbage for a text property with value '(string . ?")

[Lars Ingebrigtsen]

Alan Mackenzie <acm@muc.de> writes:

> Hello, Emacs.
>
> In the master branch:
>   emacs -Q
> .
> In the *scratch* buffer, with point at any character, do:
>
>   M-: (put-text-property (point) (1+ (point)) 'asdf '(string . ?\"))
>
> .  Without moving point,
>
>   C-u C-x =
>
> .  There appears in the *Help* buffer the following:
>
>   There are text properties here:
>     asdf                 string (widget)Top
>     face                 font-lock-comment-face
>     fontified            t
>
> .  The entry for "asdf" is garbage.  It should show the actual value of
> the text property just set, not "string (widget)Top".  The same thing
> happens in Emacs 24.5 (and presumably the emacs-25 branch).

Indeed.

> Initial investigation:
>
> The symbol 'string has a 'widget-type property.  Without validating the
> structure (or lack thereof) of the assumed widget value,
> `describe-property-list' attempts to enter the "widget browser" rather
> than printing the full value of the 'asdf text property.

`describe-property-list' is a very general-sounding function, and
doesn't mention anything about widgets, so having it do widget stuff is
rather surprising.

The only callers to this function are from descr-text and as far as I
can tell, none of them are widget-ey.  Looking at the commit log, it
looks like there was a bunch of widget stuff in here before, but it's
been removed.  Perhaps this was just forgotten?

The following trivial patch fixes the problem.  Any opinions?

diff --git a/lisp/descr-text.el b/lisp/descr-text.el
index 8be2b94458..ba53aeb385 100644
--- a/lisp/descr-text.el
+++ b/lisp/descr-text.el
@@ -88,8 +88,6 @@ describe-property-list
 	     (insert-text-button
 	      (format "%S" value)
 	      'type 'help-face 'help-args (list value)))
-            ((widgetp value)
-	     (describe-text-widget value))
 	    (t
 	     (describe-text-sexp value))))
     (insert "\n")))


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

bug#22957: C-u C-x = prints garbage for a text property with value '(string . ?")

[Alan Mackenzie]

Hello, Lars.

On Mon, Jul 29, 2019 at 15:09:10 +0200, Lars Ingebrigtsen wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > Hello, Emacs.

> > In the master branch:
> >   emacs -Q
> > .
> > In the *scratch* buffer, with point at any character, do:

> >   M-: (put-text-property (point) (1+ (point)) 'asdf '(string . ?\"))

> > .  Without moving point,

> >   C-u C-x =

> > .  There appears in the *Help* buffer the following:

> >   There are text properties here:
> >     asdf                 string (widget)Top
> >     face                 font-lock-comment-face
> >     fontified            t

> > .  The entry for "asdf" is garbage.  It should show the actual value of
> > the text property just set, not "string (widget)Top".  The same thing
> > happens in Emacs 24.5 (and presumably the emacs-25 branch).

> Indeed.

> > Initial investigation:

> > The symbol 'string has a 'widget-type property.  Without validating the
> > structure (or lack thereof) of the assumed widget value,
> > `describe-property-list' attempts to enter the "widget browser" rather
> > than printing the full value of the 'asdf text property.

> `describe-property-list' is a very general-sounding function, and
> doesn't mention anything about widgets, so having it do widget stuff is
> rather surprising.

> The only callers to this function are from descr-text and as far as I
> can tell, none of them are widget-ey.  Looking at the commit log, it
> looks like there was a bunch of widget stuff in here before, but it's
> been removed.  Perhaps this was just forgotten?

> The following trivial patch fixes the problem.  Any opinions?

It does indeed fix the problem.  I haven't tracked down anything in the
commit log, but I can believe there's no widget stuff left to drive that
cond arm.  And even if there were, it would only be a corner case (like
the original bug scenario was).

I feel only a slight unease at removing that cond arm, which would no
doubt dissipate if I took more time to look at it again.

So, I would agree with you: apply the patch and close the bug.  :-)

> diff --git a/lisp/descr-text.el b/lisp/descr-text.el
> index 8be2b94458..ba53aeb385 100644
> --- a/lisp/descr-text.el
> +++ b/lisp/descr-text.el
> @@ -88,8 +88,6 @@ describe-property-list
>  	     (insert-text-button
>  	      (format "%S" value)
>  	      'type 'help-face 'help-args (list value)))
> -            ((widgetp value)
> -	     (describe-text-widget value))
>  	    (t
>  	     (describe-text-sexp value))))
>      (insert "\n")))


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

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#22957: C-u C-x = prints garbage for a text property with value '(string . ?")

[Lars Ingebrigtsen]

Alan Mackenzie <acm@muc.de> writes:

> So, I would agree with you: apply the patch and close the bug.  :-)

OK; done.  I tried following the logic of the code in descr-text, and I
think it's the right fix.  There may be code paths I've overlooked,
though, but I guess we'll find out later.

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