bug#6830: widget-complete bad completions in :type 'file

bug#6830: widget-complete bad completions in :type 'file

[Lennart Borgman]

I get bad completion alternatives in Custom for a field of type
"file". It looks like it takes the files in the current directory and
disregards the contents of the field.

To see it start from "emacs -Q" and evaluate

(defcustom pause-later-file "~/.emacs-pause-later"
  "File for storing pauses to do later."
  :type 'file
  :group 'pause-yoga)

Then do "M-x customize-option" to customize it. Go to the file field,
enter some text and do "M-x widget-complete" (the only way to complete
it in an unpatched Emacs on w32).

With a trunk checkout from 2010-08-04 on w32.
I get a list of the file in the current dir.

bug#6830: widget-complete bad completions in :type 'file

[Chong Yidong]

Lennart Borgman <lennart.borgman@gmail.com> writes:

> I get bad completion alternatives in Custom for a field of type
> "file". It looks like it takes the files in the current directory and
> disregards the contents of the field.
>
> To see it start from "emacs -Q" and evaluate
>
> (defcustom pause-later-file "~/.emacs-pause-later"
>   "File for storing pauses to do later."
>   :type 'file
>   :group 'pause-yoga)
>
> Then do "M-x customize-option" to customize it. Go to the file field,
> enter some text and do "M-x widget-complete" (the only way to complete
> it in an unpatched Emacs on w32).

I can't reproduce any problem.  Your description is too vague; you do
not explain what "some text" is, or what happens, or why you think it is
a bug.

Please, provide a proper bug report (this is not the first time you've
been asked this).

bug#6830: widget-complete bad completions in :type 'file

[Lennart Borgman]

On Sat, Sep 4, 2010 at 7:48 PM, Chong Yidong <cyd@stupidchicken.com> wrote:
> Lennart Borgman <lennart.borgman@gmail.com> writes:
>
>> I get bad completion alternatives in Custom for a field of type
>> "file". It looks like it takes the files in the current directory and
>> disregards the contents of the field.
>>
>> To see it start from "emacs -Q" and evaluate
>>
>> (defcustom pause-later-file "~/.emacs-pause-later"
>>   "File for storing pauses to do later."
>>   :type 'file
>>   :group 'pause-yoga)
>>
>> Then do "M-x customize-option" to customize it. Go to the file field,
>> enter some text and do "M-x widget-complete" (the only way to complete
>> it in an unpatched Emacs on w32).
>
> I can't reproduce any problem.  Your description is too vague; you do
> not explain what "some text" is, or what happens, or why you think it is
> a bug.
>
> Please, provide a proper bug report (this is not the first time you've
> been asked this).


Yes, I can see now it is a bit frustrating if you do not see the bug
and think my description is not precise enough. Sorry.

I just enter any text in the file field, for example "aaa" and then
with point still in this field I do "M-x widget-complete". This gives
me all of the files in default-directory in the *Completions* buffer.

Don't you see the same? What Emacs are you testing with?

bug#6830: widget-complete bad completions in :type 'file

[Chong Yidong]

Lennart Borgman <lennart.borgman@gmail.com> writes:

> I just enter any text in the file field, for example "aaa" and then
> with point still in this field I do "M-x widget-complete". This gives
> me all of the files in default-directory in the *Completions* buffer.
>
> Don't you see the same? What Emacs are you testing with?

If I enter "~/aaa" in the widget field and there is no file starting
with "aaa" in my home directory, M-TAB does nothing except for
displaying "No match" in the echo area.

This is with the latest trunk, on GNU/Linux.

bug#6830: widget-complete bad completions in :type 'file

[Glenn Morris]

Chong Yidong wrote:

> If I enter "~/aaa" in the widget field and there is no file starting
> with "aaa" in my home directory, M-TAB does nothing except for
> displaying "No match" in the echo area.
>
> This is with the latest trunk, on GNU/Linux.

I also cannot reproduce this.
If no-one can reproduce this using stock Emacs on MS Windows, I suggest
closing this.

bug#6830: widget-complete bad completions in :type 'file

[Drew Adams]

> > If I enter "~/aaa" in the widget field and there is no file starting
> > with "aaa" in my home directory, M-TAB does nothing except for
> > displaying "No match" in the echo area.
> >
> > This is with the latest trunk, on GNU/Linux.
> 
> I also cannot reproduce this.
> If no-one can reproduce this using stock Emacs on MS Windows, 
> I suggest closing this.

I can reproduce it immediately, AFAICT.  I wonder how you actually tried to
reproduce it.

I tested for less than a minute, with emacs -Q:

In GNU Emacs 24.0.93.1 (i386-mingw-nt5.1.2600)
 of 2012-02-15 on MARVIN
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'

This seems to be similar to bug #6757 (?).

If I type a file name and then do `M-x widget-complete', exactly as the bug
recipe says, then, just as the bug report says, I get all of the files in the
current directory as completion candidates.

If I move point back one or more chars, so it is is not _after_ all of the text
typed into the entry field, then I get completions that correspond to the text
before point (more or less - see below). However, with point at some positions
in the field I get all of the files in the current directory again, and point
gets moved to the field end.

E.g. with `foo' as the text in the field, left justified, no spaces:

* With point after `foo', all files in the current dir are candidates.

* With point after `fo', only files (in dir) with _prefix_ `fo' are candidates.

* With point after `f', all files (in dir) are candidates, even though there are
some files that begin with `f' and others that do not. Furthermore, point is
automatically moved to the end of the field.  What's that about?

* With point before `f' (cursor on `f'), only files (in dir) that contain
_substring_ `fo' are candidates.

If you add some spaces before `foo', the behavior gets even weirder.  Unlike
most editing of text in Emacs (and most edit fields anywhere else), using such
an edit field is odd, because it is apparently already filled with spaces and is
in a mode similar to overwrite.  It is as if you were in picture mode.  Click in
the middle and point stays there, as if preceded by spaces.

Pretty incomprehensible behavior, if you ask me.  I'm sure there's a logical
explanation, and perhaps some or all of the behavior is even intentional (how to
know?).

However, users will never guess the behavior pattern without some explanation.
I have no idea whether all of the behavior is intentional or there are one or
more bugs.

bug#6830: widget-complete bad completions in :type 'file

[Drew Adams]

> If I type a file name and then do `M-x widget-complete', 
> exactly as the bug recipe says, then, just as the bug report
> says, I get all of the files in the current directory as
> completion candidates.
> 
> E.g. with `foo' as the text in the field, left justified, no spaces...

Similarly, for an absolute file name, such as `c:/foo'.  With point at the end
(regardless of the absolute file-name text preceding point), the completion
candidates are all of the files in the current directory, not files c:/.

If I had to guess (and guessing is all that's possible - no help available,
AFAICT), I'd guess that the text before point is ignored, so the empty input ""
is completed in `default-directory', since no absolute file name is found
_after_ point.  Most unusual behavior, in any case.

Wrt there being no help: There is a tooltip, but it says only this:
"M-TAB: complete field; RET: enter value"

Hardly helpful in trying to understand the complex completion behavior.

bug#6830: widget-complete bad completions in :type 'file

[Glenn Morris]

"Drew Adams" wrote:

> I can reproduce it immediately, AFAICT.  I wonder how you actually tried to
> reproduce it.

Using GNU/Linux. Thanks for the insinuation.

bug#6830: widget-complete bad completions in :type 'file

[Drew Adams]

> > I can reproduce it immediately, AFAICT.  I wonder how you 
> > actually tried to reproduce it.
> 
> Using GNU/Linux. Thanks for the insinuation.

No insuation.

1. By "you" I meant those people who were not able to reproduce it, starting
with Yidong.  Nothing personal intended.

2. By "how" I meant just that: what recipes you (pl.) followed.

3. You had already mentioned your platform.  That's only part of "how".

AFAICT, the bug clearly exists on MS Windows, at least.
That was my point, in response to your statement:

> If no-one can reproduce this using stock Emacs on MS Windows,
> I suggest closing this.

bug#6830: widget-complete bad completions in :type 'file

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Date: Wed, 22 Feb 2012 16:33:56 -0500
> 
> Chong Yidong wrote:
> 
> > If I enter "~/aaa" in the widget field and there is no file starting
> > with "aaa" in my home directory, M-TAB does nothing except for
> > displaying "No match" in the echo area.
> >
> > This is with the latest trunk, on GNU/Linux.
> 
> I also cannot reproduce this.
> If no-one can reproduce this using stock Emacs on MS Windows, I suggest
> closing this.

Please don't close it.  IIUC (and I'm not at all sure, as the
complexity of wid-edit.el and minibuffer.el is above my pay-grade),
the fact that it works on GNU/Linux is sheer luck.

What seems to happen is this:

 . widget-complete calls completion-in-region, which in turn calls
   minibuffer-complete, after let-binding the necessary variables:

    (let ((minibuffer-completion-table collection)
          (minibuffer-completion-predicate predicate)
          (ol (make-overlay start end nil nil t)))
      (overlay-put ol 'field 'completion)
      (when completion-in-region-mode-predicate
        (completion-in-region-mode 1)
        (setq completion-in-region--data
            (list (current-buffer) start end collection)))
      (unwind-protect
          (call-interactively 'minibuffer-complete)

 . minibuffer-complete calls completion--do-completion, which does
   this at its very beginning:

    (let* ((beg (field-beginning))
	   (end (field-end))
	   (string (buffer-substring beg end))
	   (md (completion--field-metadata beg))

   On GNU/Linux, field-beginning is correctly computed to be at the
   beginning of the field, and thus buffer-substring correctly
   extracts whatever you typed into the field, and that substring is
   thereafter correctly completed.

   In contrast, on Windows, field-beginning returns the position of
   point, so if point is at the end of whatever you typed (as it
   usually is when one hits M-TAB), buffer-substring extracts an empty
   string, which is then "completed" to the files in the
   default-directory.

The difference between the two system is, so it seems, that
find_field, which is the workhorse of field-beginning, looks at
overlays at point and before point which have the `field' property.
The problem is that we have 2 such overlays before point: one whose
`field' value is `completion' (as can be seen above, this overlay is
put there by completion-in-region), and another whose `field' value is
identical to the `field' text property at point.  Both of these
overlays have nil as their priority, so when
get_char_property_and_overlay sorts the overlays by priority, the
resulting order is arbitrary.  On GNU/Linux, the first overlay in the
sorted array happens to be the one whose `field' value is equal to the
text property, so find_field works.  On MS-Windows, the first overlay
is the one whose value is `completion', so find_field decides that the
field begins and ends at the same position.  The rest, as they say, is
history.

I hope that this analysis (if it is correct) will allow someone
smarter than myself to figure out how to fix this.

bug#6830: widget-complete bad completions in :type 'file

[Eli Zaretskii]

> From: "Drew Adams" <drew.adams@oracle.com>
> Date: Thu, 23 Feb 2012 07:27:12 -0800
> 
> > > I can reproduce it immediately, AFAICT.  I wonder how you 
> > > actually tried to reproduce it.
> > 
> > Using GNU/Linux. Thanks for the insinuation.
> 
> No insuation.
> 
> 1. By "you" I meant those people who were not able to reproduce it, starting
> with Yidong.  Nothing personal intended.
> 
> 2. By "how" I meant just that: what recipes you (pl.) followed.
> 
> 3. You had already mentioned your platform.  That's only part of "how".

You may wish to review your style, next time, and perhaps apologize
for writing something that can so easily be interpreted to mean "you
didn't really try that".

bug#6830: widget-complete bad completions in :type 'file

[Drew Adams]

> Please don't close it.  IIUC...the fact that it works
> on GNU/Linux is sheer luck. What seems to happen is this:

Thanks for this analysis.
This was far from obvious, I think.

bug#6830: widget-complete bad completions in :type 'file

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

> On GNU/Linux, the first overlay in the sorted array happens to be the
> one whose `field' value is equal to the text property, so find_field
> works.  On MS-Windows, the first overlay is the one whose value is
> `completion', so find_field decides that the field begins and ends at
> the same position.  The rest, as they say, is history.

Interesting.  The overlays code should be independent of operating
system.  Any idea what causes the sort order to be different?

Does the problem go away if in

    (let ((minibuffer-completion-table collection)
          (minibuffer-completion-predicate predicate)
          (ol (make-overlay start end nil nil t)))
      (overlay-put ol 'field 'completion)

you also specify a `priority' of (say) 5?

bug#6830: widget-complete bad completions in :type 'file

[Stefan Monnier]

>> On GNU/Linux, the first overlay in the sorted array happens to be the
>> one whose `field' value is equal to the text property, so find_field
>> works.  On MS-Windows, the first overlay is the one whose value is
>> `completion', so find_field decides that the field begins and ends at
>> the same position.  The rest, as they say, is history.
> Interesting.  The overlays code should be independent of operating
> system.  Any idea what causes the sort order to be different?

> Does the problem go away if in

>     (let ((minibuffer-completion-table collection)
>           (minibuffer-completion-predicate predicate)
>           (ol (make-overlay start end nil nil t)))
>       (overlay-put ol 'field 'completion)

> you also specify a `priority' of (say) 5?


I'm wondering what is this other overlay, why it's here, and why it ends
up sometimes taking precedence,


        Stefan

bug#6830: widget-complete bad completions in :type 'file

[Eli Zaretskii]

> From: Chong Yidong <cyd@gnu.org>
> Cc: Glenn Morris <rgm@gnu.org>,  6830@debbugs.gnu.org
> Date: Sat, 25 Feb 2012 11:30:30 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > On GNU/Linux, the first overlay in the sorted array happens to be the
> > one whose `field' value is equal to the text property, so find_field
> > works.  On MS-Windows, the first overlay is the one whose value is
> > `completion', so find_field decides that the field begins and ends at
> > the same position.  The rest, as they say, is history.
> 
> Interesting.  The overlays code should be independent of operating
> system.  Any idea what causes the sort order to be different?

I think it's quite clear: the internal details of how qsort is
implemented on each system.

> Does the problem go away if in
> 
>     (let ((minibuffer-completion-table collection)
>           (minibuffer-completion-predicate predicate)
>           (ol (make-overlay start end nil nil t)))
>       (overlay-put ol 'field 'completion)
> 
> you also specify a `priority' of (say) 5?

I didn't try, but I don't see how this would help.  The above overlay
_is_ the cause of the problem: it causes find_field to fail work as
expected.  Here's the relevant fragment of find_field:

  before_field
    = (XFASTINT (pos) > BEGV
       ? get_char_property_and_overlay (make_number (XINT (pos) - 1),
					Qfield, Qnil, NULL)
       /* Using nil here would be a more obvious choice, but it would
          fail when the buffer starts with a non-sticky field.  */
       : after_field);

  /* See if we need to handle the case where MERGE_AT_BOUNDARY is nil
     and POS is at beginning of a field, which can also be interpreted
     as the end of the previous field.  Note that the case where if
     MERGE_AT_BOUNDARY is non-nil (see function comment) is actually the
     more natural one; then we avoid treating the beginning of a field
     specially.  */
  if (NILP (merge_at_boundary))
    {
      Lisp_Object field = get_pos_property (pos, Qfield, Qnil);
      if (!EQ (field, after_field))
	at_field_end = 1;
      if (!EQ (field, before_field))
	at_field_start = 1;

The last comparison of `field' with `before_field' is the culprit: to
work correctly, they must be equal, but the above overlay causes
get_char_property_and_overlay to return `completion' instead of the
expected field value, so find_field decides we are both at the
beginning and at the end of the field, i.e. the field is empty.

Having the priority of the above overlay higher will just cause the
completion fail on GNU/Linux as well, AFAIU.  We would need this
overlay to have a _lower_ priority, for the code to DTRT, but (a) the
ELisp manual says not to use negative priorities, and (b) doing so
might very well cause trouble elsewhere, if the completion code calls
get-char-property-and-overlay and expects to get the `completion'
thing back.

bug#6830: widget-complete bad completions in :type 'file

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  6830@debbugs.gnu.org
> Date: Sat, 25 Feb 2012 00:26:05 -0500
> 
> >> On GNU/Linux, the first overlay in the sorted array happens to be the
> >> one whose `field' value is equal to the text property, so find_field
> >> works.  On MS-Windows, the first overlay is the one whose value is
> >> `completion', so find_field decides that the field begins and ends at
> >> the same position.  The rest, as they say, is history.
> > Interesting.  The overlays code should be independent of operating
> > system.  Any idea what causes the sort order to be different?
> 
> > Does the problem go away if in
> 
> >     (let ((minibuffer-completion-table collection)
> >           (minibuffer-completion-predicate predicate)
> >           (ol (make-overlay start end nil nil t)))
> >       (overlay-put ol 'field 'completion)
> 
> > you also specify a `priority' of (say) 5?
> 
> 
> I'm wondering what is this other overlay, why it's here

It's created by widget-specify-field here:

    (let ((overlay (make-overlay from to nil nil rear-sticky)))
      (widget-put widget :field-overlay overlay)
      ;;(overlay-put overlay 'detachable nil)
      (overlay-put overlay 'field widget) <<<<<<<<<<<<<<<<<<<<<<<<<<<<
      (overlay-put overlay 'local-map keymap)
      (overlay-put overlay 'face face)
      (overlay-put overlay 'follow-link follow-link)
      (overlay-put overlay 'help-echo help-echo)))

The Lisp backtrace looks like this when this is called:

  "widget-specify-field" (0x82eac8)
  "widget-setup" (0x82ed24)
  "custom-buffer-create-internal" (0x82ef94)
  "custom-buffer-create" (0x82f1e4)
  "customize-variable" (0x82f4c4)
  "call-interactively" (0x82f704)
  "execute-extended-command" (0x82f954)
  "call-interactively" (0x82fb84)

IIUC, this code is part of setting up editable fields of a widget.

> and why it ends up sometimes taking precedence,

This one I think I already explained: the order in which we get
overlays of the same priority having the same property depends on the
implementation of qsort.

In any case, looking only at the first of a list of overlays for the
same property, when we are interested in a specific value of the
property and not just for any value, is asking for trouble, I think.

bug#6830: widget-complete bad completions in :type 'file

[Richard Stallman]

One way to avoid misunderstandings is to pose questions more directly.
"I wonder how" could mean a request for information but it could also
be sarcastic.  "Would you please tell me the precise recipe for how"
is clearly a request for information.


--
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org  www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use free telephony http://directory.fsf.org/category/tel/

bug#6830: widget-complete bad completions in :type 'file

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>> Does the problem go away if in
>> 
>>     (let ((minibuffer-completion-table collection)
>>           (minibuffer-completion-predicate predicate)
>>           (ol (make-overlay start end nil nil t)))
>>       (overlay-put ol 'field 'completion)
>> 
>> you also specify a `priority' of (say) 5?
>
> I didn't try, but I don't see how this would help.  The above overlay
> _is_ the cause of the problem
> ...
> Having the priority of the above overlay higher will just cause the
> completion fail on GNU/Linux as well, AFAIU.

Sorry, I'm afraid I still don't understand.

As far as the completion code is concerned, the correct field to use is
the `completion' field assigned by completion-in-region.  In this case,
it's a mistake to pay attention to the underlying widget field.  So why
doesn't it work to give the `completion' overlay a higher priority?

FWIW, increasing the priority of the `completion' overlay does not cause
widget file name completion to fail on GNU/Linux.  Could you try on
Windows?

bug#6830: widget-complete bad completions in :type 'file

[Eli Zaretskii]

> From: Chong Yidong <cyd@gnu.org>
> Cc: rgm@gnu.org,  6830@debbugs.gnu.org
> Date: Sun, 04 Mar 2012 17:37:57 +0800
> 
> As far as the completion code is concerned, the correct field to use is
> the `completion' field assigned by completion-in-region.  In this case,
> it's a mistake to pay attention to the underlying widget field.  So why
> doesn't it work to give the `completion' overlay a higher priority?

See this fragment from find_field:

  before_field
    = (XFASTINT (pos) > BEGV
       ? get_char_property_and_overlay (make_number (XINT (pos) - 1),
					Qfield, Qnil, NULL)
       /* Using nil here would be a more obvious choice, but it would
          fail when the buffer starts with a non-sticky field.  */
       : after_field);

  /* See if we need to handle the case where MERGE_AT_BOUNDARY is nil
     and POS is at beginning of a field, which can also be interpreted
     as the end of the previous field.  Note that the case where if
     MERGE_AT_BOUNDARY is non-nil (see function comment) is actually the
     more natural one; then we avoid treating the beginning of a field
     specially.  */
  if (NILP (merge_at_boundary))
    {
      Lisp_Object field = get_pos_property (pos, Qfield, Qnil);
      if (!EQ (field, after_field))
	at_field_end = 1;
      if (!EQ (field, before_field))
	at_field_start = 1;

What do you get on GNU/Linux for the values of `field',
`before_field', and `after_field'?

> FWIW, increasing the priority of the `completion' overlay does not cause
> widget file name completion to fail on GNU/Linux.  Could you try on
> Windows?

Give me a patch to try, and I will.

bug#6830: widget-complete bad completions in :type 'file

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

> What do you get on GNU/Linux for the values of `field',
> `before_field', and `after_field'?

This is from doing M-x customize-variable RET abbrev-file-name RET, and
doing C-M-i after the end of the file name in the editable field:

(gdb) pp after_field
boundary
(gdb) pp before_field
completion
(gdb) pp field
completion

>> FWIW, increasing the priority of the `completion' overlay does not cause
>> widget file name completion to fail on GNU/Linux.  Could you try on
>> Windows?
>
> Give me a patch to try, and I will.

=== modified file 'lisp/minibuffer.el'
*** lisp/minibuffer.el	2012-02-23 15:41:12 +0000
--- lisp/minibuffer.el	2012-03-05 03:04:20 +0000
***************
*** 1483,1488 ****
--- 1483,1489 ----
            (minibuffer-completion-predicate predicate)
            (ol (make-overlay start end nil nil t)))
        (overlay-put ol 'field 'completion)
+       (overlay-put ol 'priority 5)
        (when completion-in-region-mode-predicate
          (completion-in-region-mode 1)
          (setq completion-in-region--data

bug#6830: widget-complete bad completions in :type 'file

[Eli Zaretskii]

> From: Chong Yidong <cyd@gnu.org>
> Cc: rgm@gnu.org,  6830@debbugs.gnu.org
> Date: Mon, 05 Mar 2012 11:07:44 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > What do you get on GNU/Linux for the values of `field',
> > `before_field', and `after_field'?
> 
> This is from doing M-x customize-variable RET abbrev-file-name RET, and
> doing C-M-i after the end of the file name in the editable field:
> 
> (gdb) pp after_field
> boundary
> (gdb) pp before_field
> completion
> (gdb) pp field
> completion

Then it looks like my analysis was partially incorrect: I assumed that
the problem was with the value of `before_field', but it actually is
with the value of `field'.  (Same reason, though: sorting two items
whose keys compare equal.)

> >> FWIW, increasing the priority of the `completion' overlay does not cause
> >> widget file name completion to fail on GNU/Linux.  Could you try on
> >> Windows?
> >
> > Give me a patch to try, and I will.
> 
> === modified file 'lisp/minibuffer.el'
> *** lisp/minibuffer.el	2012-02-23 15:41:12 +0000
> --- lisp/minibuffer.el	2012-03-05 03:04:20 +0000
> ***************
> *** 1483,1488 ****
> --- 1483,1489 ----
>             (minibuffer-completion-predicate predicate)
>             (ol (make-overlay start end nil nil t)))
>         (overlay-put ol 'field 'completion)
> +       (overlay-put ol 'priority 5)
>         (when completion-in-region-mode-predicate
>           (completion-in-region-mode 1)
>           (setq completion-in-region--data

Yes, this fixes the problem, thanks.

But is this a safe solution?  Can we safely assume that no code in
widget.el or its companions will ever run between overlay-put and
delete-overlay in the above function?  If some widget.el code can run
in between, it will find a value of the field property different from
what it expects.

In general, the semantics of having more than one overlay with the
same property on the same text is only clear to me when these
properties specify display features, or more generally features
related to the same parent feature.  Otherwise, it's not really clear
what the precedence means; if we use it just as a means to get the
upper hand in some use-case, we are running the risk that some other
use-case will lose.

bug#6830: widget-complete bad completions in :type 'file

[Stefan Monnier]

> Then it looks like my analysis was partially incorrect: I assumed that
> the problem was with the value of `before_field', but it actually is
> with the value of `field'.  (Same reason, though: sorting two items
> whose keys compare equal.)

Ideally, the cursor position should not be considered as "in the
boundary", so the boundary overlay should not be considered for
determining `field'.
For that, I'd guess that you'd want the boundary's marker to use the
insertion type t.  But AFAIK, the code already sets its type to t.
So I'm not sure what's going on, here.


        Stefan

bug#6830: widget-complete bad completions in :type 'file

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Chong Yidong <cyd@gnu.org>,  6830@debbugs.gnu.org
> Date: Mon, 05 Mar 2012 16:28:49 -0500
> 
> > Then it looks like my analysis was partially incorrect: I assumed that
> > the problem was with the value of `before_field', but it actually is
> > with the value of `field'.  (Same reason, though: sorting two items
> > whose keys compare equal.)
> 
> Ideally, the cursor position should not be considered as "in the
> boundary", so the boundary overlay should not be considered for
> determining `field'.
> For that, I'd guess that you'd want the boundary's marker to use the
> insertion type t.  But AFAIK, the code already sets its type to t.
> So I'm not sure what's going on, here.

I don't think I follow.  Can you elaborate on the role of the boundary
marker in this issue?

bug#6830: widget-complete bad completions in :type 'file

[Stefan Monnier]

>> > Then it looks like my analysis was partially incorrect: I assumed that
>> > the problem was with the value of `before_field', but it actually is
>> > with the value of `field'.  (Same reason, though: sorting two items
>> > whose keys compare equal.)
>> 
>> Ideally, the cursor position should not be considered as "in the
>> boundary", so the boundary overlay should not be considered for
>> determining `field'.
>> For that, I'd guess that you'd want the boundary's marker to use the
>> insertion type t.  But AFAIK, the code already sets its type to t.
>> So I'm not sure what's going on, here.

> I don't think I follow.  Can you elaborate on the role of the boundary
> marker in this issue?

I'm not sure what's the role of the boundary marker.  But from what you
say, the problem is that the boundary overlay's field property
interferes.  But the cursor should not be considered as being "in the
boundary field".  And indeed, the boundary's start marker is of
insertion-type t, which means that Fget_pos_property should ignore this
overlay when we're right in front of it.

The values that Chong sees for before_field, after_field, and field
are correct.  If in your case, `field' has a value different from
`completion' (e.g. it has value `boundary' or nil), then this is the
problem whose origin we need to find.


        Stefan

bug#6830: widget-complete bad completions in :type 'file

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: cyd@gnu.org,  6830@debbugs.gnu.org
> Date: Tue, 06 Mar 2012 15:45:02 -0500
> 
> >> Ideally, the cursor position should not be considered as "in the
> >> boundary", so the boundary overlay should not be considered for
> >> determining `field'.
> >> For that, I'd guess that you'd want the boundary's marker to use the
> >> insertion type t.  But AFAIK, the code already sets its type to t.
> >> So I'm not sure what's going on, here.
> 
> > I don't think I follow.  Can you elaborate on the role of the boundary
> > marker in this issue?
> 
> I'm not sure what's the role of the boundary marker.  But from what you
> say, the problem is that the boundary overlay's field property
> interferes.  But the cursor should not be considered as being "in the
> boundary field".  And indeed, the boundary's start marker is of
> insertion-type t, which means that Fget_pos_property should ignore this
> overlay when we're right in front of it.

Ah, that's a simple misunderstanding, then.  The boundary marker is
not related to this issue at all.  See below.

> The values that Chong sees for before_field, after_field, and field
> are correct.  If in your case, `field' has a value different from
> `completion' (e.g. it has value `boundary' or nil), then this is the
> problem whose origin we need to find.

The value of `field' on MS-Windows is neither `boundary' nor nil.  It
is the value of the widget itself.  Here's just its beginning, as an
illustration:

  (gdb) pp field
  (file :format " %v" :value "chown" :parent (custom-variable :documentation-shown t :custom-state modified :tag "Dired Chown Program" :value dired-chown-program :custom-form edit :custom-magic (custom-magic :args (nil) :parent #1 :indent 0 :children ((choice-item :help-echo "Change the state of this item." ...

As I explained earlier, this value comes from this snippet in
widget-specify-field:

    (let ((overlay (make-overlay from to nil nil rear-sticky)))
      (widget-put widget :field-overlay overlay)
      ;;(overlay-put overlay 'detachable nil)
      (overlay-put overlay 'field widget) <<<<<<<<<<<<<<<<<<<<<<<<<<<<
      (overlay-put overlay 'local-map keymap)
      (overlay-put overlay 'face face)
      (overlay-put overlay 'follow-link follow-link)
      (overlay-put overlay 'help-echo help-echo)))

When this is called as part of Customize, the Lisp backtrace looks
like this:

  "widget-specify-field" (0x82eac8)
  "widget-setup" (0x82ed24)
  "custom-buffer-create-internal" (0x82ef94)
  "custom-buffer-create" (0x82f1e4)
  "customize-variable" (0x82f4c4)
  "call-interactively" (0x82f704)
  "execute-extended-command" (0x82f954)
  "call-interactively" (0x82fb84)

IIUC, this code is part of setting up editable fields of a widget.

bug#6830: widget-complete bad completions in :type 'file

[Stefan Monnier]

> The value of `field' on MS-Windows is neither `boundary' nor nil.
> It is the value of the widget itself.  Here's just its beginning, as an
> illustration:

Oh, I think I see, now.
There are 2 overlays defining a field: one if the setup by the
completion code, the other by the widget code.  both cover the same text
area, so using either would be fine.
Now since both overlays cover the exact same area, and have no
`priority', none has priority over the other, so which of the two is
used depends on arbitrary details, e.g. implementation of the
`sort' function.

Now, until here, there's still no bug because indeed it doesn't matter
which overlay we use to determine the field.

The bug comes up because one overlay is used to get `field' and the
other is used to get `before_field'.  I.e. the "arbitrary details" end
up making a different choice.

This is probably due to the presence of the boundary overlay, so
`before_field' is computed based on the two main overlays only (because
at that position, the boundary overlay is absent), whereas for `field',
we take the overlays at the end pos, where we have the two main overlays
plus the boundary overlay, so the sort is applied to a different list of
overlays, resulting in a different ordering of the two overlays we
care about.

Does the patch below fix the problem?


        Stefan


=== modified file 'src/buffer.c'
--- src/buffer.c	2012-03-02 15:57:16 +0000
+++ src/buffer.c	2012-03-07 22:08:13 +0000
@@ -2864,7 +2864,11 @@
     return s1->beg < s2->beg ? -1 : 1;
   if (s1->end != s2->end)
     return s2->end < s1->end ? -1 : 1;
-  return 0;
+  /* Avoid the non-determinism of qsort by choosing an arbitrary ordering
+     between "equal" overlays.  The result can still change between
+     invocations of Emacs, but it won't change in the middle of
+     `find_field' (bug#6830).  */
+  return XHASH (s1->overlay) < XHASH (s2->overlay) ? -1 : 1;
 }
 
 /* Sort an array of overlays by priority.  The array is modified in place.

bug#6830: widget-complete bad completions in :type 'file

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: cyd@gnu.org,  6830@debbugs.gnu.org
> Date: Wed, 07 Mar 2012 17:09:56 -0500
> 
> > The value of `field' on MS-Windows is neither `boundary' nor nil.
> > It is the value of the widget itself.  Here's just its beginning, as an
> > illustration:
> 
> Oh, I think I see, now.
> There are 2 overlays defining a field: one if the setup by the
> completion code, the other by the widget code.  both cover the same text
> area

Yes.

> so using either would be fine.

??? I don't understand this part.  It maybe OK in this particular
case, because all you care about is the range of buffer positions
covered by the overlay.  But in general, the value of the field
property is presumably of at least some importance to the Lisp
application that examines the field.  So feeding the application by
some other overlay, even if covers the same text, is generally unsafe,
don't you agree?  Or am I missing something?

> Now since both overlays cover the exact same area, and have no
> `priority', none has priority over the other, so which of the two is
> used depends on arbitrary details, e.g. implementation of the
> `sort' function.

Right.

> Now, until here, there's still no bug because indeed it doesn't matter
> which overlay we use to determine the field.

See above.  I'm not sure this is correct in general.  And what if one
of the overlays covers more buffer positions than the other?  Wouldn't
it sometimes confuse the completion code?

> Does the patch below fix the problem?

It does, but on Windows both `before_field' and `field' now have the
value of the widget, not of `completion'.

More generally, the overlay returned by get-char-property-and-overlay
in this situation is still arbitrary after your change, and the Lisp
application still has no control on which one will be returned.  So
same issue of having more than one `field' overlay on the same buffer
position still remains.

bug#6830: widget-complete bad completions in :type 'file

[Stefan Monnier]

>> so using either would be fine.
> ??? I don't understand this part.  It maybe OK in this particular
> case, because all you care about is the range of buffer positions
> covered by the overlay.

That's normally the case.  If an application actually cares about which
value of `field' shows up, it'll have to do more work.

>> Now, until here, there's still no bug because indeed it doesn't matter
>> which overlay we use to determine the field.
> See above.  I'm not sure this is correct in general.  And what if one
> of the overlays covers more buffer positions than the other?  Wouldn't
> it sometimes confuse the completion code?

The normal priority rules say that a smaller overlay normally takes
priority, but yes, if there's a small field already defined within the
completion region, then the completion code can become confused.
I don't know how likely this is.

>> Does the patch below fix the problem?
> It does, but on Windows both `before_field' and `field' now have the
> value of the widget, not of `completion'.
> More generally, the overlay returned by get-char-property-and-overlay
> in this situation is still arbitrary after your change, and the Lisp
> application still has no control on which one will be returned.

It has the same control as before: the use of the `priority' property.


        Stefan

bug#6830: widget-complete bad completions in :type 'file

[Stefan Monnier]

>> Does the patch below fix the problem?
> It does, but on Windows both `before_field' and `field' now have the
> value of the widget, not of `completion'.

Thanks, installed,


        Stefan

bug#6830: widget-complete bad completions in :type 'file

[Paul Eggert]

That patch assumes that s1 != s2.  This assumption about qsort
is quite likely true on Windows but it's not guaranteed by
the C standard, and for portability it's better to play it safe
and stick with the pattern that's already in the code.
I installed this further patch:

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2012-03-12 06:34:32 +0000
+++ src/ChangeLog	2012-03-12 08:27:25 +0000
@@ -1,3 +1,7 @@
+2012-03-12  Paul Eggert  <eggert@cs.ucla.edu>
+
+	* buffer.c (compare_overlays): Don't assume args differ (Bug#6830).
+
 2012-03-12  Chong Yidong  <cyd@gnu.org>
 
 	* eval.c (inhibit_lisp_code): Rename from

=== modified file 'src/buffer.c'
--- src/buffer.c	2012-03-09 16:46:25 +0000
+++ src/buffer.c	2012-03-12 08:27:25 +0000
@@ -2868,7 +2868,9 @@
      between "equal" overlays.  The result can still change between
      invocations of Emacs, but it won't change in the middle of
      `find_field' (bug#6830).  */
-  return XHASH (s1->overlay) < XHASH (s2->overlay) ? -1 : 1;
+  if (XHASH (s1->overlay) != XHASH (s2->overlay))
+    return XHASH (s1->overlay) < XHASH (s2->overlay) ? -1 : 1;
+  return 0;
 }
 
 /* Sort an array of overlays by priority.  The array is modified in place.