Re: project--completing-read-strict breaks ada-mode project completion table

[Stephen Leake]

Stefan Monnier <address@hidden> writes:

>> Yes, because a collection should only specify a style if it cannot work
>> with other styles.
>
> I read this to mean that this collection-specified style would only be
> used by collections which are fundamentally "broken" (because unable to
> work as collection for any other style).

Yes.

Note that uniq-file-completion-table is an optimization; the
uniquify-file completion style does work with a list of absolute files,
or any completion table function that returns such a list.

> I had the impression that such "broken" collections are very rare.

Ok. Possibly also undesirable/discouraged?

The optimization provided by uniq-file-completion-table (as opposed to a
list of all files) is important. Maybe I can improve that function so it
works with other styles; more below.

path-iterator.el could provide a completion table function that
implements the same optimizations, which would work with existing
styles.

> We definitely want to support the case where the collection works OK for
> most/many/some styles, where it also wants to provide its own style, and
> where we want the user to be able to override that (via
> completion-category-oiverrides).  So even for those collections where
> only its own style works, it makes sense to use that same (overridable)
> mechanism rather than provide another.

But then project backends that choose to provide a uniquify-file (or any
other "broken") completion table won't "just work"; the user will have
to set completion-category-default or -overrides to get completion to
work (as illustrated by locate-uniquified-file; more below).

>>> I don't understand this.  Why can't the completion style compute the
>>> common-prefix?
>> The root is _not_ the common prefix of the current matches;
>
> I don't mean "of the current matches" but "of the entire collection".
> (try-completion "" collection) should return just that, so the
> completion-style does have access to it (unless the collection doesn't
> implement `try-completion` correctly for that case, but it seems that
> in the present situation we have control over the collection so we can
> make sure it works correctly).

Yes, that would work, and be horribly inefficient. The initial delay in
computing the file list for the emacs source code is already noticeable.
Rescanning the entire list on every completion step would be unacceptable.

>>> Also, I don't quite understand why we need 2: they both seem to
>>> implement the same direction of the conversion (i.e. from "user-string"
>>> to "data-string").
>>
>> No, "table input format" is not the same as "data format". See the
>> header comments in uniquify-files; "table input" is the format required
>> for the "string" parameter to the completion table functions; for both
>> uniquify-file and file-root-rel it is "partial_dir/partial_basename".
>> "data" is the format returned by all-completions; an absolute file name.
>
> IIUC this means you're using a different API than the normal
> completion-table API (e.g. in the normal API the return value of
> (try-completion STR COLL) should have STR as prefix and (all-completions
> STR COLL) should have as prefix the part of STR past the
> completion-boundary).
>
> So that means the interface between your collection and your
> completion-style is basically private and hence your 3 representation
> and the functions between them are also internal implementation
> choices.

I hope this is a extension that could be useful in other situations.

Since uniquify-file style works with a plain list collection, your
suggestion is not completely true.

Ah; note in completion-to-table-input, it converts user to table input
format if the table is a function, but to data format if it's a list.
Which means if I change uniq-file-completion-table to accept data format
strings, it should work with other styles.

uniq-file-completion-table must accept both data and table format
strings, since converting user to data requires searching the table,
passing a partial directory path (ie table input format). It can check
to see if the search string is an absolute file name, and adapt
accordingly if needed (ie the regex can be anchored or not).

> BTW, I have some problems when trying your code: I started Emacs, loaded
> uniquify-files and then did
>
>     M-: (locate-uniquified-file '("/usr/bin"))
>
> and when I type
>
>     d?
>
> in the minibuffer, I get the list of matching *absolute* files in
> *Completions*.

This is a symptom of the problem discussed above; you need to set either
completion-category-overrides or -defaults to specify uniquify-files to
make this work properly.

One way to do that is in locate-uniquified-file, as I did for
project--completing-read-strict:

(defun locate-uniquified-file (&optional path predicate default prompt)
  (interactive)
  (let* ((iter (make-path-iterator :user-path-non-recursive (or path 
load-path)))
         (table (apply-partially #'uniq-file-completion-table iter))
         (table-styles (cdr (assq 'styles (completion-metadata "" table nil))))
         (completion-category-overrides
          (list (list 'project-file (cons 'styles table-styles)))))
    (completing-read (or prompt "file: ")
                     table predicate t nil nil default)
    ))

In an early version of uniquify-files, it modified
completion-category-defaults for project-files when loaded, so
locate-uniquified-file worked. But when I added the file-root-rel style,
it became clear that needs to be a user choice, not hard-coded.

Setting completion-category-overrides in locate-uniquified-file is ok,
since the name indicates uniquified style.

> What I meant is that when we call uniq-file-get-data-string we already
> know that the user string is a valid match, so there shouldn't be any
> need to search in the completion table.  E.g. if it's
> "foo-file1.text<Alice/alice-1/>" we should be able to just prepend the
> hidden common prefix to the output of uniq-file-to-table-input, no?

Yes, but that common prefix is only stored implicitly in the table.

If we had a completion object, we could store it there.

Given recent experiments with string properties, it might be possible to
store the abs file name as a string property. But I doubt that will work
unless we fix minibuffer.el to fully implement
minibuffer-allow-text-properties (as you suggest below).

> Later you wrote:
>> This is now implemented, in ELPA.
> [...]
>> Except it also needs:
>> (setq minibuffer-allow-text-properties t)
> [...]
>
> And then:
>> That's not enough. Despite that setting, when there is a single
>> completion, minibuffer-force-complete calls completion--replace,
>
> We could fix completion--replace.  It currently removes all
> properties, but that's just because it was easier, I think it only cares
> about visible properties, so we could be more careful to only remove the
> `face`, `invisible`, and `display` properties.

Right.

>> In addition, minibuffer-force-complete unconditionally uses
>> buffer-substring-no-properties to return the text.
>
> We could also try and change this.

Right. There are probably other things that need to be fixed.

> But thinking more about it, relying on text-properties is inconvenient
> and unnecessary: inconvenient because as we can see they tend to be
> unreliable (and there's always the chance that the user typed that part
> of the text anyway), and unnecessary because this kind of completion
> style (which rearranges the text of completion candidates) can't really
> be mixed with other styles (the resulting behavior would be rather
> weird),

I guess by "mixing with other styles" you mean processing a list of
styles; if we had (basic uniquify-files), for <tab> on an empty string
'basic' would present a list of absolute file names, but for <tab> after
a string that matches some basename 'uniquify-files' would present
uniquified file names. That would be confusing, and I now realize that's
why I only specify one completion style. And it explains in more detail
what's happening with locate-uniquified-files.

> I'm starting to wonder if "completion-style" is the right place
> to put this.  E.g. after removing the common prefix and swapping the
> dir/file to file/dir the remaining completion operations could
> meaningfully use completion-styles, so maybe it's better thought as an
> extra layer between the completion style and the collection ...
> ... or something.

The core requirements are:

- completing-read return an absolute file name, from a collection of abs
  file names.

- display mimimum directory info in completion options

  or more generally, user format /= data format


Maybe there could be a "uniquify/minimize" step when computing the
display list from the result of all-completion. Then all styles would
call uniq-file--uniquify (but only for project-file or file category!?
not useful for buffers and symbols).

Note that uniq-file--uniquify is currently called _almost_ at the end of
uniq-file-all-completions; uniq-file--hilit is called after it. hilit is
tightly tied to both the style and user string format.

If uniq-file--uniquify preserved string properties (I have no idea if it
does curerntly), hilit could be called first. That would be required to
work with other styles.

There would have to be a matching "de-uniquify/maximize" step.

It will probably require editing minibuffer.el, but I'll think about it.

--
-- Stephe