bug#17052: substitute-command-keys strips text properties

bug#17052: substitute-command-keys strips text properties

[Juanma Barranquero]

Package: emacs
Severity: wishlist

Currently, substitute-command-keys returns a string with text
properties stripped. This should be fixed.

One specific case where it matters is in docstrings of advised
functions, as discussed in the thread for bug#16993.

bug#17052: substitute-command-keys strips text properties

[Lars Ingebrigtsen]

Juanma Barranquero <lekktu@gmail.com> writes:

> Currently, substitute-command-keys returns a string with text
> properties stripped. This should be fixed.
>
> One specific case where it matters is in docstrings of advised
> functions, as discussed in the thread for bug#16993.

(I'm going through old bug reports that have unfortunately gotten no
responses yet.)

Looks like this has been fixed sometime in the intervening years?

(substitute-command-keys (propertize "foo" 'face 'bold))
=> #("foo" 0 3 (face bold))

So I'm closing this bug report.  If this is still a problem, please
reopen.

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

bug#17052: substitute-command-keys strips text properties

[Glenn Morris]

Lars Ingebrigtsen wrote:

> Looks like this has been fixed sometime in the intervening years?
>
> (substitute-command-keys (propertize "foo" 'face 'bold))
> => #("foo" 0 3 (face bold))

Nope, you need to use an example which does a substitution, eg

(substitute-command-keys (propertize "foo \\[find-file]" 'face 'bold))

The doc of substitute-command-keys used to mention this, but it was
somewhat prematurely removed in 049fac7.

bug#17052: substitute-command-keys strips text properties

[Lars Ingebrigtsen]

Glenn Morris <rgm@gnu.org> writes:

> Lars Ingebrigtsen wrote:
>
>> Looks like this has been fixed sometime in the intervening years?
>>
>> (substitute-command-keys (propertize "foo" 'face 'bold))
>> => #("foo" 0 3 (face bold))
>
> Nope, you need to use an example which does a substitution, eg
>
> (substitute-command-keys (propertize "foo \\[find-file]" 'face 'bold))

Ah, I see.  I've now reopened the bug report again.

> The doc of substitute-command-keys used to mention this, but it was
> somewhat prematurely removed in 049fac7.

Heh, heh:

    (Fsubstitute_command_keys): Don't advertise the fact that
    text-properties are dropped, since we think it's a bug that we'll fix
    in 24.5.

Should I just put it back in the doc string, or do we still think that
we'll fix it?

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

bug#17052: substitute-command-keys strips text properties

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Wed, 14 Aug 2019 18:58:57 -0700
> Cc: Juanma Barranquero <lekktu@gmail.com>, 17052@debbugs.gnu.org
> 
> > The doc of substitute-command-keys used to mention this, but it was
> > somewhat prematurely removed in 049fac7.
> 
> Heh, heh:
> 
>     (Fsubstitute_command_keys): Don't advertise the fact that
>     text-properties are dropped, since we think it's a bug that we'll fix
>     in 24.5.
> 
> Should I just put it back in the doc string, or do we still think that
> we'll fix it?

I don't think it matters (unless you want to fix this right now ;-).
We should document the situation and leave the bug open for the
possibility that someone would like to work on this.

Thanks.

bug#17052: substitute-command-keys strips text properties

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> Heh, heh:
>> 
>>     (Fsubstitute_command_keys): Don't advertise the fact that
>>     text-properties are dropped, since we think it's a bug that we'll fix
>>     in 24.5.
>> 
>> Should I just put it back in the doc string, or do we still think that
>> we'll fix it?
>
> I don't think it matters (unless you want to fix this right now ;-).

I had a look at that function now and, er, no.  :-)  Perhaps it should
be rewritten in Emacs Lisp?  Then it'd be pretty trivial to fix this
stuff.

> We should document the situation and leave the bug open for the
> possibility that someone would like to work on this.

Document the situation by just putting back into the doc string that it
doesn't copy text properties in some situations?

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

bug#17052: substitute-command-keys strips text properties

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: rgm@gnu.org,  17052@debbugs.gnu.org,  lekktu@gmail.com
> Date: Thu, 15 Aug 2019 15:42:18 -0700
> 
> > We should document the situation and leave the bug open for the
> > possibility that someone would like to work on this.
> 
> Document the situation by just putting back into the doc string that it
> doesn't copy text properties in some situations?

That's what I meant, yes.  Is there a problem with that which I
missed?

bug#17052: substitute-command-keys strips text properties

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Lars Ingebrigtsen <larsi@gnus.org>
>> Cc: rgm@gnu.org,  17052@debbugs.gnu.org,  lekktu@gmail.com
>> Date: Thu, 15 Aug 2019 15:42:18 -0700
>> 
>> > We should document the situation and leave the bug open for the
>> > possibility that someone would like to work on this.
>> 
>> Document the situation by just putting back into the doc string that it
>> doesn't copy text properties in some situations?
>
> That's what I meant, yes.  Is there a problem with that which I
> missed?

No, I was just wondering whether you meant documenting it in some other
way since my question was

>>> Should I just put it back in the doc string, or do we still think that
>>> we'll fix it?

and your answer wasn't "just put it back in the doc string".  :-)

Which I'll now do, but leave the bug report open.

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

bug#17052: substitute-command-keys strips text properties

[Stefan Kangas]

[In reply to Bug#17052 and cross-posting to Bug#8951.]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I had a look at that function now and, er, no.  :-)  Perhaps it should
> be rewritten in Emacs Lisp?  Then it'd be pretty trivial to fix this
> stuff.

The Fsubstitute_command_keys function is indeed a bit hairy, and I
agree that a conversion to Lisp is the most reasonable next step.

I've actually been working a patch to convert it to Lisp for Bug#8951,
but it's not finished yet.  The difficulty is that it has a couple of
helper functions that also needs to be lifted, which makes the job
bigger than just the function Fsubstitute_command_keys.

My goal is to get the Lisp version to produce the same output as the C
version, which currently seems to be a feasible goal.  Once it's in
Lisp it should be easier to make further improvements, fix bugs, etc.

Thanks,
Stefan Kangas

bug#17052: substitute-command-keys strips text properties

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sun, 18 Aug 2019 19:08:18 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>, 17052@debbugs.gnu.org, lekktu@gmail.com, 
> 	8951@debbugs.gnu.org
> 
> > I had a look at that function now and, er, no.  :-)  Perhaps it should
> > be rewritten in Emacs Lisp?  Then it'd be pretty trivial to fix this
> > stuff.
> 
> The Fsubstitute_command_keys function is indeed a bit hairy, and I
> agree that a conversion to Lisp is the most reasonable next step.
> 
> I've actually been working a patch to convert it to Lisp for Bug#8951,
> but it's not finished yet.  The difficulty is that it has a couple of
> helper functions that also needs to be lifted, which makes the job
> bigger than just the function Fsubstitute_command_keys.
> 
> My goal is to get the Lisp version to produce the same output as the C
> version, which currently seems to be a feasible goal.  Once it's in
> Lisp it should be easier to make further improvements, fix bugs, etc.

Maybe we should first talk a bit about why Lars you think it would be
"pretty trivial" to keep properties in Lisp, but not in C.  I'm
probably missing something very basic here, because I don't see why
it would be easier in Lisp.

bug#17052: substitute-command-keys strips text properties

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> I've actually been working a patch to convert it to Lisp for Bug#8951,
> but it's not finished yet.  The difficulty is that it has a couple of
> helper functions that also needs to be lifted, which makes the job
> bigger than just the function Fsubstitute_command_keys.
>
> My goal is to get the Lisp version to produce the same output as the C
> version, which currently seems to be a feasible goal.  Once it's in
> Lisp it should be easier to make further improvements, fix bugs, etc.

Sounds great!

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

bug#17052: substitute-command-keys strips text properties

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Maybe we should first talk a bit about why Lars you think it would be
> "pretty trivial" to keep properties in Lisp, but not in C.  I'm
> probably missing something very basic here, because I don't see why
> it would be easier in Lisp.

Anything is easier in Lisp, surely.

But if you look at that function in particular, it's written in a very
C-ey fashion, what with copying over characters one by one etc.  It
could be rewritten (in C) to use the proper primitives we have that
preserve text properties (Fsubstring etc), but then you'd just end up
with a more long-winded version that would be Lisp-in-C, so we might as
well just rewrite it in Lisp.

It'll probably be a fraction of the code length.

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

bug#17052: substitute-command-keys strips text properties

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Stefan Kangas <stefan@marxist.se>,  17052@debbugs.gnu.org,
>   lekktu@gmail.com,  8951@debbugs.gnu.org
> Date: Sun, 18 Aug 2019 15:43:35 -0700
> 
> But if you look at that function in particular, it's written in a very
> C-ey fashion, what with copying over characters one by one etc.  It
> could be rewritten (in C) to use the proper primitives we have that
> preserve text properties (Fsubstring etc), but then you'd just end up
> with a more long-winded version that would be Lisp-in-C, so we might as
> well just rewrite it in Lisp.

AFAIU, the problematic parts are not those which copy characters one
by one, the problematic parts are those which _substitute_ one text
with another.  In that case, you need to adjust the text properties or
cons them out of thin air.

bug#17052: substitute-command-keys strips text properties

[Stefan Kangas]

close 17052 28.1
thanks

Glenn Morris <rgm@gnu.org> writes:

>> Looks like this has been fixed sometime in the intervening years?
>>
>> (substitute-command-keys (propertize "foo" 'face 'bold))
>> => #("foo" 0 3 (face bold))
>
> Nope, you need to use an example which does a substitution, eg
>
> (substitute-command-keys (propertize "foo \\[find-file]" 'face 'bold))

This was fixed by the recent rewrite of substitute-command-keys in Lisp.
I've added a test for this in commit 27655f9f38, and am therefore
closing this bug.

(For posterity, the root cause for this was that `princ' does not
preserve text properties.  The code now uses `insert' instead.)

bug#17052: substitute-command-keys strips text properties, bug#8911: huh?

[Drew Adams]

> close 17052 28.1
> thanks

I don't understand the relation between this bug, #17052,
and bug #8911.  Half of the messages in the 8911 thread
are about 17052, and the subjects seem quite different.

But it looks like 8911 is still open (wishlist).  Is that
correct?  Why/how did these two bugs get overlapped or
combined?

bug#17052: substitute-command-keys strips text properties, bug#8911: huh?

[Stefan Kangas]

Drew Adams <drew.adams@oracle.com> writes:

> I don't understand the relation between this bug, #17052,
> and bug #8911.  Half of the messages in the 8911 thread
> are about 17052, and the subjects seem quite different.
>
> But it looks like 8911 is still open (wishlist).  Is that
                    ^^^^ you probably mean 8951
> correct?  Why/how did these two bugs get overlapped or
> combined?

It was my perhaps ill-advised attempt to combine the discussions about
the Lisp conversion of `substitute-command-keys'.

(FWIW, I intend to look into Bug#8951 soon-ish.)

bug#17052: substitute-command-keys strips text properties, bug#8911: huh?

[Drew Adams]

> > I don't understand the relation between this bug, #17052,
> > and bug #8911.  Half of the messages in the 8911 thread
> > are about 17052, and the subjects seem quite different.
> >
> > But it looks like 8911 is still open (wishlist).  Is that correct?
>                     ^^^^ you probably mean 8951

Yes, sorry; I meant 8951.

> > Why/how did these two bugs get overlapped or combined?
> 
> It was my perhaps ill-advised attempt to combine the discussions about
> the Lisp conversion of `substitute-command-keys'.
> 
> (FWIW, I intend to look into Bug#8951 soon-ish.)