GNU bug report logs - #8795
24.0.50; `completion-try-completion' addition of METADATA arg

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 8795 in the body.
You can then email your comments to 8795 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: <bug-gnu-emacs <at> gnu.org>
Subject: 24.0.50; `completion-try-completion' addition of METADATA arg
Date: Fri, 3 Jun 2011 10:44:58 -0700

1. The addition of arg METADATA, without making it optional, breaks
existing code that calls the function with only the first four args.
 
What value of METADATA should be passed to get the equivalent of the
behavior prior to the addition of this arg?  nil?
 
How about making this arg optional, so user code does not need to
special-case Emacs 24 wrt Emacs 23?


2. The METADATA arg is not even described in the doc string.
Users have no way to know how to fix the code this breaks.
Please describe the arg in the doc string.

In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
 of 2011-05-31 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.5) --no-opt --cflags
-Ic:/build/include'
 

Message #8 received at 8795 <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: <8795 <at> debbugs.gnu.org>
Subject: RE: bug#8795: 24.0.50;
	`completion-try-completion' addition of METADATA arg
Date: Fri, 3 Jun 2011 12:27:09 -0700

Same problem for `completion-all-completions'.

Message #11 received at 8795 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: "Drew Adams" <drew.adams <at> oracle.com>
Cc: 8795 <at> debbugs.gnu.org
Subject: Re: bug#8795: 24.0.50;
	`completion-try-completion' addition of METADATA arg
Date: Mon, 06 Jun 2011 11:49:13 -0300

> 1. The addition of arg METADATA, without making it optional, breaks
> existing code that calls the function with only the first four args.
 
Indeed, it was meant as an internal function, tho clearly it was never
fully internal since icomplete uses it.

I'll see how I can fix it, but in the mean time, could you tell me how
you use it (I can guess the"where" is "icicles")?


        Stefan

Message #14 received at 8795 <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Stefan Monnier'" <monnier <at> iro.umontreal.ca>
Cc: 8795 <at> debbugs.gnu.org
Subject: RE: bug#8795: 24.0.50;
	`completion-try-completion' addition of METADATA arg
Date: Mon, 6 Jun 2011 08:29:51 -0700

> > 1. The addition of arg METADATA, without making it optional, breaks
> > existing code that calls the function with only the first four args.
>  
> Indeed, it was meant as an internal function, tho clearly it was never
> fully internal since icomplete uses it.
> 
> I'll see how I can fix it, but in the mean time, could you tell me how
> you use it (I can guess the "where" is "icicles")?

I use it here:

1. icomplete+.el
http://www.emacswiki.org/emacs/icomplete%2b.el
in the Emacs 23+ definition of `icomplete-completions':

(let* ((mdata  (and (fboundp 'completion--field-metadata)
                    (completion--field-metadata
                     (field-beginning))))
       (most-try
        (if (fboundp 'completion--field-metadata) ; Emacs 24
            (completion-try-completion
             name comps nil (length name) mdata)
          (completion-try-completion
           name comps nil (length name))))
...

2. icicles-fn.el
http://www.emacswiki.org/emacs/icicles-fn.el
in the definitions of `icicle-completion-(all|try)-completion(s)':

(let* ((mdata  (and (fboundp 'completion--field-metadata)
                    (or metadata  (completion--field-metadata
                                   (field-beginning)))))
       (res    (if (fboundp 'completion--field-metadata) ; Emacs 24
                   (completion-all-completions
                    string table pred point mdata)
                 (completion-all-completions
                  string table pred point))))
...

[similarly for `...try...']

Functions `icicle-completion-(all|try)-completion(s)' are used by functions
`icicle-unsorted(-file-name)-prefix-candidates',
`icicle-prefix-any(-file-name)-candidates-p' (in the same file).

Here is a typical use:

(if (icicle-not-basic-prefix-completion-p)
    (icicle-completion-try-completion
     input minibuffer-completion-table
     minibuffer-completion-predicate
     (length input)
     (and (fboundp 'completion--field-metadata)
          (completion--field-metadata ; Emacs 24
           (field-beginning))))
  (try-completion input minibuffer-completion-table
                  minibuffer-completion-predicate))

`icicle-not-basic-prefix-completion-p' returns non-nil if the user chooses to
take advantage of Emacs 23+ `completion-styles' etc.

Message #19 received at 8795-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: 8795-done <at> debbugs.gnu.org
Subject: Re: bug#8795: 24.0.50;
	`completion-try-completion' addition of METADATA arg
Date: Mon, 20 Jun 2011 16:16:52 -0400

> 1. The addition of arg METADATA, without making it optional, breaks
> existing code that calls the function with only the first four args.
 
It's now optional,


        Stefan

Message #20 received at 8795-done <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Stefan Monnier'" <monnier <at> iro.umontreal.ca>, <8795-done <at> debbugs.gnu.org>
Subject: RE: bug#8795: 24.0.50;
	`completion-try-completion' addition of METADATA arg
Date: Mon, 20 Jun 2011 13:48:15 -0700

> It's now optional,

Thanks.

Message #21 received at 8795-done <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Stefan Monnier'" <monnier <at> iro.umontreal.ca>, <8795-done <at> debbugs.gnu.org>
Subject: RE: bug#8795: 24.0.50;
	`completion-try-completion' addition of METADATA arg
Date: Mon, 20 Jun 2011 15:28:32 -0700

> It's now optional,

You closed the bug, but apparently you fixed only #1: make the METADATA
parameter optional, not #2: describe the parameter.

I still don't see METADATA described in the doc strings of these two functions.

I looked here:
http://bzr.savannah.gnu.org/lh/emacs/trunk/annotate/head:/lisp/minibuffer.el

Thx.

Message #34 received at 8795-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: "Drew Adams" <drew.adams <at> oracle.com>
Cc: 8795-done <at> debbugs.gnu.org
Subject: Re: bug#8795: 24.0.50;
	`completion-try-completion' addition of METADATA arg
Date: Wed, 12 Oct 2011 17:28:30 -0400

> Still no doc for arg METADATA.

Indeed.  I have no intention to add any doc for it.  It's obvious from
context, and this is an internal function.


        Stefan

Message #35 received at 8795-done <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Stefan Monnier'" <monnier <at> iro.umontreal.ca>
Cc: 8795-done <at> debbugs.gnu.org
Subject: RE: bug#8795: 24.0.50;
	`completion-try-completion' addition of METADATA arg
Date: Wed, 12 Oct 2011 15:09:02 -0700

> > Still no doc for arg METADATA.
> 
> Indeed.  I have no intention to add any doc for it.
> It's obvious from context, and this is an internal function.

The bug should not be closed as fixed/done.  "Wontfix" is what you are saying.

And it is clearly not internal.  If it were internal then you would not have
needed to change METADATA to `&optional'.  And I was not the only one to point
out that its not being optional broke backward compatibility.

http://lists.gnu.org/archive/html/emacs-devel/2011-06/msg00327.html
http://lists.gnu.org/archive/html/emacs-devel/2011-06/msg00348.html

I use it in my code, and obviously others do too.  IIRC, it was you who
suggested that I use it instead of `try-completion', at least in some contexts
(or possibly it was c*-all-c* vs all-c*, or both - anyway, I use both). [*]

Its parameters, including METADATA, should be documented.  And no, it is not at
all "obvious from context" what could or should be passed as METADATA, and when.
The meaning and behavior should be described.  Is that so hard to do?

----
[*] I use it in code such as this (similar, for c*-all-c*):
(when (consp filtered-candidates)
  (let ((common-prefix
         (if (icicle-not-basic-prefix-completion-p)
             (icicle-completion-try-completion ; <=========
              input minibuffer-completion-table
              minibuffer-completion-predicate
              (length input)
              (and (fboundp 'completion--field-metadata)
                   (completion--field-metadata ; Emacs 24
                    (field-beginning))))
           (try-completion input minibuffer-completion-table
                           minibuffer-completion-predicate))))
    (setq icicle-common-match-string
          (if (eq t common-prefix) input common-prefix))))

`icicle-c*-try-c*' is nearly the same as `c*-try-c*', but with bug #4708 fixed
(so it returns nil when there is no such file etc.)

(defun icicle-completion-try-completion (string table pred point
                                         &optional metadata)
  "Icicles version of `completion-try-completion'.
1. Handle all Emacs versions.
2. Remove the last cdr, which might hold the base size.
3. METADATA is optional and defaults to `completion--field-metadata'
   at `field-beginning'."
  (let* ((mdata  (and (fboundp 'completion--field-metadata)
                      (or metadata
                          (completion--field-metadata
                           (field-beginning)))))
         (res    (if (fboundp 'completion--field-metadata)
                     (completion-try-completion ; <=========
                      string table pred point mdata)
                   (completion-try-completion   ; <=========
                    string table pred point))))
    (when (consp res) (setq res  (car res)))
    res))

If I can easily simplify some of this I would be glad to hear how.

Message #36 received at 8795-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: "Drew Adams" <drew.adams <at> oracle.com>
Cc: 8795-done <at> debbugs.gnu.org
Subject: Re: bug#8795: 24.0.50;
	`completion-try-completion' addition of METADATA arg
Date: Wed, 12 Oct 2011 22:21:41 -0400

> And it is clearly not internal.

It is.  "Internal" reflects the intention behind this function.  As the
author of the code, I know better than you do whether it's internal or not.

> If it were internal then you would not have needed to change METADATA
> to `&optional'.  And I was not the only one to point out that its not
> being optional broke backward compatibility.

Oh, so because I was nice enough to go through the trouble to preserve
backward compatibility, I am now bound to additionally document
the obvious because you don't find it obvious enough?


        Stefan

Message #37 received at 8795-done <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Stefan Monnier'" <monnier <at> iro.umontreal.ca>
Cc: 8795-done <at> debbugs.gnu.org
Subject: RE: bug#8795: 24.0.50;
	`completion-try-completion' addition of METADATA arg
Date: Thu, 13 Oct 2011 07:14:58 -0700

> > And it is clearly not internal.
> 
> It is.  "Internal" reflects the intention behind this 
> function.  As the author of the code, I know better than
> you do whether it's internal or not.

Your intention is one thing.  Reality (actual use) is apparently another.

You already acknowledged that you didn't think anyone would ever use this - you
were surprised that people do (have to?).  What you intended as internal, not
realizing the impact of your changes, is not internal - in practice.

Do you think people jump through hoops like this just because they want to
complicate things, with different code for different Emacs versions?

;; Recent Emacs
(completion-try-completion
  input minibuffer-completion-table
  minibuffer-completion-predicate
  (length input)
  (completion--field-metadata (field-beginning)))

instead of just

;; Older Emacs
(try-completion input minibuffer-completion-table
                minibuffer-completion-predicate))

How much simpler it would be to just use `try-completion' here for all Emacs
versions. 

As I said, "If I can easily simplify some of this I would be glad to hear how."
No one _wants_ to use something you intended as internal, if they don't have to.

> > If it were internal then you would not have needed to 
> > change METADATA to `&optional'.  And I was not the only one
> > to point out that its not being optional broke backward compatibility.
> 
> Oh, so because I was nice enough to go through the trouble to preserve
> backward compatibility, I am now bound to additionally document
> the obvious because you don't find it obvious enough?

Backward compatibility takes care of, well, backward compatibility.  It does not
take care of documenting how to use the new (more complex) paraphernalia.

Again, why not?  Is it so hard to explain the meaning and behavior of the
METADATA parameter?  What's wrong with describing even an "internal" function?
If you so strongly resist putting the info in a doc string, then put it in a
code comment, at least.  Code comments are how we document "internal" functions,
no?

Whether something like this is obvious enough is not for you to determine, but
for readers of the code.  An author can vouch for the author's _intention_, but
not for how well the meaning and behavior are communicated.

Message #40 received at 8795 <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Stefan Monnier'" <monnier <at> iro.umontreal.ca>
Cc: 8795-done <at> debbugs.gnu.org, 8795 <at> debbugs.gnu.org
Subject: RE: bug#8795: 24.0.50;
	`completion-try-completion' addition of METADATA arg
Date: Thu, 13 Oct 2011 13:36:41 -0700

And if it were really internal then you would not, yourself, need to use it also
in other libraries, such as `icomplete.el'.

And that's the other place where I (need to) use it too: `icomplete.el+', which
tweaks the `icomplete.el' code.

Message #44 received at 8795 <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: <8795 <at> debbugs.gnu.org>
Subject: FW: bug#8795: 24.0.50;
	`completion-try-completion' addition of METADATA arg
Date: Thu, 13 Oct 2011 13:45:39 -0700

> > And it is clearly not internal.
> 
> It is.  "Internal" reflects the intention behind this 
> function.  As the author of the code, I know better than
> you do whether it's internal or not.

Your intention is one thing.  Reality (actual use) is apparently another.

You already acknowledged that you didn't think anyone would ever use this - you
were surprised that people do (have to?).  What you intended as internal, not
realizing the impact of your changes, is not internal - in practice.

Do you think people jump through hoops like this just because they want to
complicate things, with different code for different Emacs versions?

;; Recent Emacs
(completion-try-completion
  input minibuffer-completion-table
  minibuffer-completion-predicate
  (length input)
  (completion--field-metadata (field-beginning)))

instead of just

;; Older Emacs
(try-completion input minibuffer-completion-table
                minibuffer-completion-predicate))

How much simpler it would be to just use `try-completion' here for all Emacs
versions. 

As I said, "If I can easily simplify some of this I would be glad to hear how."
No one _wants_ to use something you intended as internal, if they don't have to.

> > If it were internal then you would not have needed to 
> > change METADATA to `&optional'.  And I was not the only one
> > to point out that its not being optional broke backward compatibility.
> 
> Oh, so because I was nice enough to go through the trouble to preserve
> backward compatibility, I am now bound to additionally document
> the obvious because you don't find it obvious enough?

Backward compatibility takes care of, well, backward compatibility.  It does not
take care of documenting how to use the new (more complex) paraphernalia.

Again, why not?  Is it so hard to explain the meaning and behavior of the
METADATA parameter?  What's wrong with describing even an "internal" function?
If you so strongly resist putting the info in a doc string, then put it in a
code comment, at least.  Code comments are how we document "internal" functions,
no?

Whether something like this is obvious enough is not for you to determine, but
for readers of the code.  An author can vouch for the author's _intention_, but
not for how well the meaning and behavior are communicated.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.