all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [pcomplete.el (pcomplete-completions-at-point)] Why max?
@ 2019-03-16 22:29 Tadeus Prastowo
  2019-03-19  9:55 ` Tadeus Prastowo
  2019-03-20  2:09 ` Stefan Monnier
  0 siblings, 2 replies; 11+ messages in thread
From: Tadeus Prastowo @ 2019-03-16 22:29 UTC (permalink / raw)
  To: John Wiegley; +Cc: emacs-devel

Hi John,

I am using Emacs at commit 4633b0e and see a problem that can be
solved ad-hoc by using `min' instead of `max' in the following code
within function `pcomplete-completions-at-point' in file
`lisp/pcomplete.el':

           (beg (max (- (point) (length pcomplete-stub))
                     (pcomplete-begin)))

What do you think will break if `min' is used instead of `max' to
repair the following problem seen using `emacs -Q' at the said commit?

M-x shell
cd /tmp
mkdir AAAA\ BB\ CCCC
cd AAAA\ BB<tab>

Autocomplete fails because (pcomplete-begin) returns the position of
the first letter A but (length pcomplete-stub) is the length of "AAAA
BB", which gives the position of the second letter A.  The function
`max', therefore, sets `beg' to the start of the second letter A.
Consequently, file-name-completion will be asked to complete "AAA BB"
instead of the correct one "AAAA BB".

Thank you.

--
Best regards,
Tadeus



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pcomplete.el (pcomplete-completions-at-point)] Why max?
  2019-03-16 22:29 [pcomplete.el (pcomplete-completions-at-point)] Why max? Tadeus Prastowo
@ 2019-03-19  9:55 ` Tadeus Prastowo
  2019-03-19 16:37   ` Stefan Monnier
  2019-03-20  2:09 ` Stefan Monnier
  1 sibling, 1 reply; 11+ messages in thread
From: Tadeus Prastowo @ 2019-03-19  9:55 UTC (permalink / raw)
  To: emacs-devel; +Cc: John Wiegley

Hi everyone,

Does John still maintain the pcomplete package?

--
Best regards,
Tadeus

On Sat, Mar 16, 2019 at 11:29 PM Tadeus Prastowo
<tadeus.prastowo@unitn.it> wrote:
>
> Hi John,
>
> I am using Emacs at commit 4633b0e and see a problem that can be
> solved ad-hoc by using `min' instead of `max' in the following code
> within function `pcomplete-completions-at-point' in file
> `lisp/pcomplete.el':
>
>            (beg (max (- (point) (length pcomplete-stub))
>                      (pcomplete-begin)))
>
> What do you think will break if `min' is used instead of `max' to
> repair the following problem seen using `emacs -Q' at the said commit?
>
> M-x shell
> cd /tmp
> mkdir AAAA\ BB\ CCCC
> cd AAAA\ BB<tab>
>
> Autocomplete fails because (pcomplete-begin) returns the position of
> the first letter A but (length pcomplete-stub) is the length of "AAAA
> BB", which gives the position of the second letter A.  The function
> `max', therefore, sets `beg' to the start of the second letter A.
> Consequently, file-name-completion will be asked to complete "AAA BB"
> instead of the correct one "AAAA BB".
>
> Thank you.
>
> --
> Best regards,
> Tadeus



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pcomplete.el (pcomplete-completions-at-point)] Why max?
  2019-03-19  9:55 ` Tadeus Prastowo
@ 2019-03-19 16:37   ` Stefan Monnier
  2019-03-19 16:40     ` Tadeus Prastowo
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2019-03-19 16:37 UTC (permalink / raw)
  To: emacs-devel

> Does John still maintain the pcomplete package?

Not sure, but in any case, I'm responsible for this part of the code
(i.e. for the bridge between pcomplete and completion-at-point).

I haven't had time to look at your problem yet, but it's there on my
todo ;-)


        Setfan




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pcomplete.el (pcomplete-completions-at-point)] Why max?
  2019-03-19 16:37   ` Stefan Monnier
@ 2019-03-19 16:40     ` Tadeus Prastowo
  0 siblings, 0 replies; 11+ messages in thread
From: Tadeus Prastowo @ 2019-03-19 16:40 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

On Tue, Mar 19, 2019 at 5:38 PM Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> > Does John still maintain the pcomplete package?
>
> Not sure, but in any case, I'm responsible for this part of the code
> (i.e. for the bridge between pcomplete and completion-at-point).
>
> I haven't had time to look at your problem yet, but it's there on my
> todo ;-)

Cool, thank you very much for replying.

>         Setfan

--
Best regards,
Tadeus



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pcomplete.el (pcomplete-completions-at-point)] Why max?
  2019-03-16 22:29 [pcomplete.el (pcomplete-completions-at-point)] Why max? Tadeus Prastowo
  2019-03-19  9:55 ` Tadeus Prastowo
@ 2019-03-20  2:09 ` Stefan Monnier
  2019-03-20 11:00   ` Tadeus Prastowo
  1 sibling, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2019-03-20  2:09 UTC (permalink / raw)
  To: emacs-devel

> M-x shell
> cd /tmp
> mkdir AAAA\ BB\ CCCC
> cd AAAA\ BB<tab>

When I try this (with Emacs `master` but AFAIK this hasn't changed for
quite a while), I get the expected completion to

    cd AAAA\ BB\ CCCC

what do you get instead?

> Autocomplete fails because (pcomplete-begin) returns the position of
> the first letter A but (length pcomplete-stub) is the length of "AAAA
> BB", which gives the position of the second letter A.  The function
> `max', therefore, sets `beg' to the start of the second letter A.
> Consequently, file-name-completion will be asked to complete "AAA BB"
> instead of the correct one "AAAA BB".

The `completion-table-subvert` and `completion-table-with-quoting`
layers of the completion table are supposed to convert the "AAA\ BB" to
"AAAA BB" and back, so that file-name-completion should see neither
"AAAA\ BB" nor "AAA BB" but "AAAA BB" (which is indeed the string it
needs to do its job correctly).

> What do you think will break if `min' is used instead of `max' to
> repair the following problem seen using `emacs -Q' at the said commit?

To some extent the value of `beg` is not tremendously important because
of the `completion-table-subvert` layer, but there are cases where it
does make a difference.  I can't offhand remember enough to tell you
which are those cases (IIRC it has to do with cases where completion
wants to change text *before* point, e.g. completing `em-li` to
`emacs-lisp` or /u/s/d to /usr/share/doc) so I can't quite remember when
`min` would be worse than `max` here, but IIRC `beg` is used as a kind
of "modification boundary" (the completion operation cannot modify any
part of the text before `beg`) so I use `max` to minimize the damage in
case the replacement breaks the assumptions made by
`completion-table-subvert`.

[ As alluded to in the comment just before the code you cite, there's
  a fundamental discrepancy between the information that pcomplete
  collects and the information that completion-at-point-function needs,
  so we do our best to workaround and confine the cases where the
  discrepancy bites us.  ]


        Stefan




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pcomplete.el (pcomplete-completions-at-point)] Why max?
  2019-03-20  2:09 ` Stefan Monnier
@ 2019-03-20 11:00   ` Tadeus Prastowo
  2019-03-20 14:00     ` Stefan Monnier
  0 siblings, 1 reply; 11+ messages in thread
From: Tadeus Prastowo @ 2019-03-20 11:00 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

On Wed, Mar 20, 2019 at 3:13 AM Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> > M-x shell
> > cd /tmp
> > mkdir AAAA\ BB\ CCCC
> > cd AAAA\ BB<tab>
>
> When I try this (with Emacs `master` but AFAIK this hasn't changed for
> quite a while), I get the expected completion to
>
>     cd AAAA\ BB\ CCCC
>
> what do you get instead?

Okay, that magic word does not work.  Please try the following magic
word (I try it at commit 047c1b19353ff58d8cd45935c7b44c911b70e312):

$ emacs -Q
M-x shell
cd /tmp
mkdir ABCD\ EF\ GHIJKL
cd ABCD\ EF<tab>

The echo area shows "No match".  With my proposed fix (not yet
adjusted to the said commit, I just did C-M-x on my proposed patch),
the completion works.

The correct magic word seems to depend on the correct combinations of
the number of characters and their cases.  The occurrence of the magic
work in the wild is not rare, though, since I had hit this problem
several times till I was compelled last weekend to debug it.

What do you think the real problem is?

> > Autocomplete fails because (pcomplete-begin) returns the position of
> > the first letter A but (length pcomplete-stub) is the length of "AAAA
> > BB", which gives the position of the second letter A.  The function
> > `max', therefore, sets `beg' to the start of the second letter A.
> > Consequently, file-name-completion will be asked to complete "AAA BB"
> > instead of the correct one "AAAA BB".
>
> The `completion-table-subvert` and `completion-table-with-quoting`
> layers of the completion table are supposed to convert the "AAA\ BB" to
> "AAAA BB" and back, so that file-name-completion should see neither
> "AAAA\ BB" nor "AAA BB" but "AAAA BB" (which is indeed the string it
> needs to do its job correctly).

Okay, I will keep it in mind.

> > What do you think will break if `min' is used instead of `max' to
> > repair the following problem seen using `emacs -Q' at the said commit?
>
> To some extent the value of `beg` is not tremendously important because
> of the `completion-table-subvert` layer, but there are cases where it
> does make a difference.  I can't offhand remember enough to tell you
> which are those cases (IIRC it has to do with cases where completion
> wants to change text *before* point, e.g. completing `em-li` to
> `emacs-lisp` or /u/s/d to /usr/share/doc) so I can't quite remember when
> `min` would be worse than `max` here, but IIRC `beg` is used as a kind
> of "modification boundary" (the completion operation cannot modify any
> part of the text before `beg`) so I use `max` to minimize the damage in
> case the replacement breaks the assumptions made by
> `completion-table-subvert`.
>
> [ As alluded to in the comment just before the code you cite, there's
>   a fundamental discrepancy between the information that pcomplete
>   collects and the information that completion-at-point-function needs,
>   so we do our best to workaround and confine the cases where the
>   discrepancy bites us.  ]

Thank you very much for your information.  I will keep it in mind.

>         Stefan

--
Best regards,
Tadeus



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pcomplete.el (pcomplete-completions-at-point)] Why max?
  2019-03-20 11:00   ` Tadeus Prastowo
@ 2019-03-20 14:00     ` Stefan Monnier
  2019-03-20 16:14       ` Tadeus Prastowo
  0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2019-03-20 14:00 UTC (permalink / raw)
  To: Tadeus Prastowo; +Cc: emacs-devel

> $ emacs -Q
> M-x shell
> cd /tmp
> mkdir ABCD\ EF\ GHIJKL
> cd ABCD\ EF<tab>
>
> The echo area shows "No match".

I see it now!
I just pushed the fix below which seems to do the trick!


        Stefan


 * lisp/minibuffer.el (completion-table-subvert): Fix typo from rev 5697ca55cb

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index df0acbb343..dbd24dfa0a 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -269,7 +269,7 @@ completion-table-subvert
                     (+ beg (- (length s1) (length s2))))
               . ,(and (eq (car-safe res) 'boundaries) (cddr res)))))
          ((stringp res)
-          (if (string-prefix-p s2 string completion-ignore-case)
+          (if (string-prefix-p s2 res completion-ignore-case)
               (concat s1 (substring res (length s2)))))
          ((eq action t)



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [pcomplete.el (pcomplete-completions-at-point)] Why max?
  2019-03-20 14:00     ` Stefan Monnier
@ 2019-03-20 16:14       ` Tadeus Prastowo
  2019-03-20 17:11         ` Stefan Monnier
  0 siblings, 1 reply; 11+ messages in thread
From: Tadeus Prastowo @ 2019-03-20 16:14 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

On Wed, Mar 20, 2019 at 3:00 PM Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> > $ emacs -Q
> > M-x shell
> > cd /tmp
> > mkdir ABCD\ EF\ GHIJKL
> > cd ABCD\ EF<tab>
> >
> > The echo area shows "No match".
>
> I see it now!
> I just pushed the fix below which seems to do the trick!

I confirm that commit 18fc4ac5294d85e37d9e544c04b5d4e89ef3237c solves
the problem.

Why the problem seems to be dependent on the number of characters and
their cases?

Thank you very much.

>         Stefan

--
Best regards,
Tadeus

>  * lisp/minibuffer.el (completion-table-subvert): Fix typo from rev 5697ca55cb
>
> diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
> index df0acbb343..dbd24dfa0a 100644
> --- a/lisp/minibuffer.el
> +++ b/lisp/minibuffer.el
> @@ -269,7 +269,7 @@ completion-table-subvert
>                      (+ beg (- (length s1) (length s2))))
>                . ,(and (eq (car-safe res) 'boundaries) (cddr res)))))
>           ((stringp res)
> -          (if (string-prefix-p s2 string completion-ignore-case)
> +          (if (string-prefix-p s2 res completion-ignore-case)
>                (concat s1 (substring res (length s2)))))
>           ((eq action t)



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pcomplete.el (pcomplete-completions-at-point)] Why max?
  2019-03-20 16:14       ` Tadeus Prastowo
@ 2019-03-20 17:11         ` Stefan Monnier
  2019-03-20 17:17           ` Tadeus Prastowo
  2019-03-20 23:38           ` John Wiegley
  0 siblings, 2 replies; 11+ messages in thread
From: Stefan Monnier @ 2019-03-20 17:11 UTC (permalink / raw)
  To: Tadeus Prastowo; +Cc: emacs-devel

> I confirm that commit 18fc4ac5294d85e37d9e544c04b5d4e89ef3237c solves
> the problem.
> Why the problem seems to be dependent on the number of characters and
> their cases?

It's because when we

    (string-prefix-p s2 string completion-ignore-case)
    
instead of

    (string-prefix-p s2 res completion-ignore-case)

we still get the correct result if `string` also happens to begin with
`s2` (in your cases, `s2` is short, typically the same number of chars
as the number of backslashes in your argument).


        Stefan



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pcomplete.el (pcomplete-completions-at-point)] Why max?
  2019-03-20 17:11         ` Stefan Monnier
@ 2019-03-20 17:17           ` Tadeus Prastowo
  2019-03-20 23:38           ` John Wiegley
  1 sibling, 0 replies; 11+ messages in thread
From: Tadeus Prastowo @ 2019-03-20 17:17 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

On Wed, Mar 20, 2019 at 6:11 PM Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> > I confirm that commit 18fc4ac5294d85e37d9e544c04b5d4e89ef3237c solves
> > the problem.
> > Why the problem seems to be dependent on the number of characters and
> > their cases?
>
> It's because when we
>
>     (string-prefix-p s2 string completion-ignore-case)
>
> instead of
>
>     (string-prefix-p s2 res completion-ignore-case)
>
> we still get the correct result if `string` also happens to begin with
> `s2` (in your cases, `s2` is short, typically the same number of chars
> as the number of backslashes in your argument).

I see.  Thank you very much.

Lastly, do you mind closing the following bug report by saying that it
is fixed at commit 18fc4ac5294d85e37d9e544c04b5d4e89ef3237c, please?

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=34888

Thanks in advance.

>         Stefan

--
Best regards,
Tadeus



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [pcomplete.el (pcomplete-completions-at-point)] Why max?
  2019-03-20 17:11         ` Stefan Monnier
  2019-03-20 17:17           ` Tadeus Prastowo
@ 2019-03-20 23:38           ` John Wiegley
  1 sibling, 0 replies; 11+ messages in thread
From: John Wiegley @ 2019-03-20 23:38 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Tadeus Prastowo, emacs-devel

>>>>> "SM" == Stefan Monnier <monnier@iro.umontreal.ca> writes:

SM> It's because when we

SM>     (string-prefix-p s2 string completion-ignore-case)

SM> instead of

SM>     (string-prefix-p s2 res completion-ignore-case)

SM> we still get the correct result if `string` also happens to begin with
SM> `s2` (in your cases, `s2` is short, typically the same number of chars as
SM> the number of backslashes in your argument).

Thank you, Stefan!

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-03-20 23:38 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-16 22:29 [pcomplete.el (pcomplete-completions-at-point)] Why max? Tadeus Prastowo
2019-03-19  9:55 ` Tadeus Prastowo
2019-03-19 16:37   ` Stefan Monnier
2019-03-19 16:40     ` Tadeus Prastowo
2019-03-20  2:09 ` Stefan Monnier
2019-03-20 11:00   ` Tadeus Prastowo
2019-03-20 14:00     ` Stefan Monnier
2019-03-20 16:14       ` Tadeus Prastowo
2019-03-20 17:11         ` Stefan Monnier
2019-03-20 17:17           ` Tadeus Prastowo
2019-03-20 23:38           ` John Wiegley

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.