bug#14153: 24.3.50; regression: `crm--choose-completion-string' expects wrong args

bug#14153: 24.3.50; regression: `crm--choose-completion-string' expects wrong args

[Drew Adams]

A regression introduced in Emacs 23.  There is no problem before 23.

`completing-read-multiple' adds `crm--choose-completion-string' to
`choose-completion-string-functions'.
 
But `crm--choose-completion-string' has the wrong signature for a
function on `choose-completion-string-functions'.
 
Each such function should accept 4 args, as follows:
 
 CHOICE - the string to insert in the buffer,
 BUFFER - the buffer in which the choice should be inserted,
 MINI-P - non-nil if BUFFER is a minibuffer, and
 BASE-SIZE - the number of characters in BUFFER before
          the string being completed.
 
But `crm--choose-completion-string' expects these args:
 
 (CHOICE  BUFFER  BASE-POSITION  &rest IGNORED)

In particular, it expects the third arg to be a position,
not a Boolean indicator of whether BUFFER is a minibuffer.
 
If the BUFFER is a minibuffer, so that MINI-P is non-nil
(e.g. `t'), then `crm--choose-completion-string'
mistakenly interprets that value as the BASE-POSITION.
 
In code that I have this leads to this kind of error:
 
Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p t)
  choose-completion-string("mode-line" #<buffer  *Minibuf-1*> t)
  crm--choose-completion-string("mode-line" #<buffer  *Minibuf-1*> t nil)
  run-hook-with-args-until-success(crm--choose-completion-string "mode-line"
                                    #<buffer  *Minibuf-1*> t nil)
 
Presumably no one has run into this because their code hasn't passed t
as the value of MINI-P (?).

 
In GNU Emacs 24.3.50.1 (i386-mingw-nt5.1.2600)
 of 2013-04-02 on ODIEONE
Bzr revision: 112212 cyd@gnu.org-20130402033331-sqegwhqh7u1o0ars
Windowing system distributor `Microsoft Corp.', version 5.1.2600
Configured using:
 `configure --with-gcc (4.7) --no-opt --enable-checking --cflags
 -IC:/Devel/emacs/build/include --ldflags -LC:/Devel/emacs/build/lib'
 

bug#14153: 24.3.50; regression: `crm--choose-completion-string' expects wrong args

[Stefan Monnier]

> Each such function should accept 4 args, as follows:
 
>  CHOICE - the string to insert in the buffer,
>  BUFFER - the buffer in which the choice should be inserted,
>  MINI-P - non-nil if BUFFER is a minibuffer, and
>  BASE-SIZE - the number of characters in BUFFER before
>           the string being completed.

Duh, sorry indeed, could you please update this docstring to match the code?

> But `crm--choose-completion-string' expects these args:
>  (CHOICE  BUFFER  BASE-POSITION  &rest IGNORED)

Which is indeed the correct calling convention nowadays.


        Stefan

bug#14153: 24.3.50; regression: `crm--choose-completion-string' expects wrong args

[Drew Adams]

> > Each such function should accept 4 args, as follows:
>  
> >  CHOICE - the string to insert in the buffer,
> >  BUFFER - the buffer in which the choice should be inserted,
> >  MINI-P - non-nil if BUFFER is a minibuffer, and
> >  BASE-SIZE - the number of characters in BUFFER before
> >           the string being completed.
> 
> Duh, sorry indeed, could you please update this docstring to 
> match the code?

Sorry, I don't know what the new code parameters mean.  S?he who changes the
signature is best placed to update the spec/doc of that signature.

> > But `crm--choose-completion-string' expects these args:
> >  (CHOICE  BUFFER  BASE-POSITION  &rest IGNORED)
> 
> Which is indeed the correct calling convention nowadays.

How do you deal with older code that calls with a MINI-P parameter?

It's one thing to ignore old parameters that you don't need anymore.  It's quite
another thing to change the signature so much that old calls provoke errors.

This is apparently not about some particular function such as
`crm--choose-completion-string'.  It is about
`choose-completion-string-functions', which has been around a long time and is a
general purpose hook.

Anyway, since you've changed the code, please change the doc string accordingly,
whatever that might mean/entail.  Thx.  If you really need help with the
wording, then communicate the meaning (content), and I will be glad to suggest
wording.

bug#14153: 24.3.50; regression: `crm--choose-completion-string' expects wrong args

[Drew Adams]

I now see an entry in the Emacs 23 NEWS about this (good), now that I know what
the change is, so what to look for (`choose-completion-string-functions').

bug#14153: 24.3.50; regression: `crm--choose-completion-string' expects wrong args

[Lars Ingebrigtsen]

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

>> Each such function should accept 4 args, as follows:
>
>>  CHOICE - the string to insert in the buffer,
>>  BUFFER - the buffer in which the choice should be inserted,
>>  MINI-P - non-nil if BUFFER is a minibuffer, and
>>  BASE-SIZE - the number of characters in BUFFER before
>>           the string being completed.
>
> Duh, sorry indeed, could you please update this docstring to match the code?
>
>> But `crm--choose-completion-string' expects these args:
>>  (CHOICE  BUFFER  BASE-POSITION  &rest IGNORED)
>
> Which is indeed the correct calling convention nowadays.

Fixed on trunk.

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