bug#12377: 24.2.50; `minibuffer-prompt-properties' should respect existing faces in prompt string

bug#12377: 24.2.50; `minibuffer-prompt-properties' should respect existing faces in prompt string

[Drew Adams]

`minibuffer-prompt-properties', via `minibuffer-prompt' face, overrides
a `face' property that might exist on certain parts of the prompt
string.  This is not good.  Emacs should be less overbearing and more
respectful of prompt strings, respecting existing properties, particular
`face', that might be present.

In GNU Emacs 24.2.50.1 (i386-mingw-nt5.1.2600)
 of 2012-09-02 on MARVIN
Bzr revision: 109861 eggert@cs.ucla.edu-20120902171035-7mzihil3xd6bjfiy
Windowing system distributor `Microsoft Corp.', version 5.1.2600
Configured using:
 `configure --with-gcc (4.6) --no-opt --enable-checking --cflags
 -ID:/devel/emacs/libs/libXpm-3.5.8/include
 -ID:/devel/emacs/libs/libXpm-3.5.8/src
 -ID:/devel/emacs/libs/libpng-dev_1.4.3-1/include
 -ID:/devel/emacs/libs/zlib-dev_1.2.5-2/include
 -ID:/devel/emacs/libs/giflib-4.1.4-1/include
 -ID:/devel/emacs/libs/jpeg-6b-4/include
 -ID:/devel/emacs/libs/tiff-3.8.2-1/include
 -ID:/devel/emacs/libs/gnutls-3.0.9/include
 -ID:/devel/emacs/libs/libiconv-1.13.1-1-dev/include
 -ID:/devel/emacs/libs/libxml2-2.7.8/include/libxml2'
 

bug#12377: 24.2.50; `minibuffer-prompt-properties' should respect existing faces in prompt string

[Lars Ingebrigtsen]

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

> `minibuffer-prompt-properties', via `minibuffer-prompt' face, overrides
> a `face' property that might exist on certain parts of the prompt
> string.  This is not good.  Emacs should be less overbearing and more
> respectful of prompt strings, respecting existing properties, particular
> `face', that might be present.

Yeah.  Perhaps it should use `add-face-text-property' instead of
overwriting the text properties?  Any objections?

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

bug#12377: 24.2.50; `minibuffer-prompt-properties' should respect existing faces in prompt string

[Drew Adams]

> Yeah.  Perhaps it should use `add-face-text-property' instead of
> overwriting the text properties?  Any objections?

Yes.  If the prompt already has a `face' text property then assume
that is because the user or some other code wants to use that face.

Do not replace or add any `face' property to a prompt that already
has a `face' property.  This should be a no-brainer, IMO.

bug#12377: 24.2.50; `minibuffer-prompt-properties' should respect existing faces in prompt string

[Lars Ingebrigtsen]

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

> `minibuffer-prompt-properties', via `minibuffer-prompt' face, overrides
> a `face' property that might exist on certain parts of the prompt
> string.  This is not good.  Emacs should be less overbearing and more
> respectful of prompt strings, respecting existing properties, particular
> `face', that might be present.

Do you have a recipe that exhibits this bug?

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

bug#12377: 24.2.50; `minibuffer-prompt-properties' should respect existing faces in prompt string

[Drew Adams]

> Do you have a recipe that exhibits this bug?

C'mon Lars.  Please read the bug thread, including your own
contribution to it.  This should be a no-brainer.

emacs -Q

(completing-read (propertize "My prompt:" 'face 'highlight) '(a b c d))

bug#12377: 24.2.50; `minibuffer-prompt-properties' should respect existing faces in prompt string

[Lars Ingebrigtsen]

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

> (completing-read (propertize "My prompt:" 'face 'highlight) '(a b c d))

See, it's easy to create test cases if you just try.  Well done.

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

bug#12377: 24.2.50; `minibuffer-prompt-properties' should respect existing faces in prompt string

[Lars Ingebrigtsen]

It's unfortunate that read_minibuf uses this way to apply the minibuf
text properties:

	Fadd_text_properties (make_number (BEG), make_number (PT),
			      Vminibuffer_prompt_properties, Qnil);

Or rather, the design of minibuffer-prompt-properties.  It includes one
property that's pretty important for the prompt, namely `read-only', and
one that's for display only, namely `face'.  And the latter is something
that you'd want to be able to override by saying `propertize' on the
prompt, definitely.

I'm inclined to think that if the user has set any `face' properties on
the prompt, then read_minibuf should avoid setting it.  We could just
loop over the minibuffer-prompt-properties list and apply the ones that
aren't set any place in the string already?  Does that sound OK to
everybody?

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

bug#12377: 24.2.50; `minibuffer-prompt-properties' should respect existing faces in prompt string

[Drew Adams]

Dunno why this needs to be done in C code anyway.

It is OK for Emacs to make the prompt read-only, I guess (i.e., to not let user code make it writable), though I would prefer that it just do that by default.

It is OK for Emacs to apply face `minibuffer-prompt' or whatever, by default.

It is not OK for Emacs to overwrite any text properties.

In general, user code should be allowed to do anything it wants to the prompt text. But Emacs should provide, by default, a read-only prompt with its standard face.

And it should be done in Lisp, preferably, so users can easily modify it at will.  This is Emacs.  There is nothing critical for performance about setting up the minibuffer prompt string.

bug#12377: 24.2.50; `minibuffer-prompt-properties' should respect existing faces in prompt string

[Lars Ingebrigtsen]

I've now implemented this on the trunk -- the faces are merged with the
face specified by -prompt-properties (which are at the end), as
suggested by Stefan.

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

bug#12377: 24.2.50; `minibuffer-prompt-properties' should respect existing faces in prompt string

[Drew Adams]

> I've now implemented this on the trunk -- the faces are merged with the
> face specified by -prompt-properties (which are at the end), as
> suggested by Stefan.

There was nothing posted to this bug thread by Stefan.  Likewise
for the other bug, #16136, with which this one was merged.

The above post by Lars was in reply to this:

>> Do you have a recipe that exhibits this bug?
> emacs -Q
> (completing-read (propertize "My prompt:" 'face 'highlight) '(a b c d))

Please reopen this bug.  It is NOT fixed.  At least not in this
build from 5/25, which is long after the "fix" was implemented:

In GNU Emacs 25.0.94.1 (x86_64-w64-mingw32)
 of 2016-05-25 built on KAEL
Windowing system distributor 'Microsoft Corp.', version 6.1.7601
Configured using:
 'configure --prefix=/tmp/emacs --without-imagemagick
 --enable-checking=yes,glyphs --enable-check-lisp-object-type
 'CFLAGS=-Og -gdwarf-4 -g3''

You still cannot do what was requested, including the obvious
as illustrated by the above `completing-read' test case.

bug#12377: 24.2.50; `minibuffer-prompt-properties' should respect existing faces in prompt string

[Noam Postavsky]

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

On Fri, Jun 24, 2016 at 12:33 PM, Drew Adams <drew.adams@oracle.com> wrote:
>> I've now implemented this on the trunk -- the faces are merged with the
>> face specified by -prompt-properties (which are at the end)
[...]
> The above post by Lars was in reply to this:
>
>>> Do you have a recipe that exhibits this bug?
>> emacs -Q
>> (completing-read (propertize "My prompt:" 'face 'highlight) '(a b c d))
>
> Please reopen this bug.  It is NOT fixed.  At least not in this
> build from 5/25, which is long after the "fix" was implemented:
>
> In GNU Emacs 25.0.94.1 (x86_64-w64-mingw32)
>  of 2016-05-25 built on KAEL

It's fixed in master (the git equivalent of "trunk"), which
corresponds to versions 25.1.50+, see attached screenshot.

[-- Attachment #2: propertized-prompt.png --]
[-- Type: image/png, Size: 17187 bytes --]

bug#12377: 24.2.50; `minibuffer-prompt-properties' should respect existing faces in prompt string

[Drew Adams]

> It's fixed in master (the git equivalent of "trunk"), which
> corresponds to versions 25.1.50+, see attached screenshot.

OK, good to know; thanks.  That May 25 pretest build is the latest
I have.