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

[Stephen Leake]

Stefan Monnier <address@hidden> writes:

>>       (table-styles (cdr (assq 'styles (completion-metadata "" collection 
>> nil))))
>>       (completion-category-overrides
>>        (list (list 'project-file (cons 'styles
>>                                        (or table-styles
>>                                            
>> project-file-completion-styles)))))
>>
>>       ;; If the completion table is a list, or a function that does
>>       ;; not return styles metadata, we set completion-styles to
>>       ;; reflect the user choice.
>>       (completion-styles (if table-styles nil 
>> project-file-completion-styles))
>
> This gives precedence to the collection's styles with no way for the
> user to override this choice. 

Yes, because a collection should only specify a style if it cannot work
with other styles. For example, if you comment out the styles metadata
in uniq-file-completion-table (line 717 in uniquify-files.el), and set

(uniquify-test-project-completion-style 'uniquify-file)
(project-file-completion-styles '(substring)),

the completion does not work properly; you can complete on the basename,
but not on the directory part.

Similarly for the styles metadata in fc-root-rel-completion-table-iter.

I need to come up with a concise explanation of when a completion table
should return a style.

In this demo, there are two user-setable variables to indicate
completion style; uniquify-test-project-completion-style at the project
backend level, project-file-completion-styles at the generic project
level. The user can set either, so they always have a choice. That
should be a general rule; if a project backend provides a completion
table that requires a certain style, it should also provide a plain list
so the user can choose the style.

> This problem is the reason why I designed the current system to go
> through the indirection of a category, making it possible for the user
> to override the category's default styles via
> completion-category-overrides.
>
>> The uniquify-file completion style works on a list of files; see the
> [...]
>> However, file-root-rel does not, because the root directory must be
>> stored somewhere that is accessible from the completion code.
>
> 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; it's an
arbitrary directory, nominally the single project root.

In project--completing-read-strict, it's the common prefix of the entire
collection, computed once at the start.

If the style computed the common prefix on each match set, it might not
be constant during the completion; it could get longer as the choices
are narrowed. That does not happen in project--completing-read-strict in
master, nor in file-root-rel.

>> uniquify-files.el now adds two functions to completion-styles-alist, for
>> converting strings from user to table input format, or user to data
>> format.
>
> As I mentioned in another message to João, I think we should move from
> completion-style-alist to using generic functions that dispatch on the
> style.

That makes sense.

> 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.

> I see that uniq-file-get-data-string does more (i.e. it tries to find
> the corresponding match if there's only one) but I don't understand
> why you need to do that: this seems to do a kind of completion which
> minibuffer-complete-and-exit should already have done (if
> `require-match` is set) or which shouldn't be done (if `require-match`
> is not set).

The main computation in uniq-file-get-data-string is to return the
absolute file name corresponding to the table input string.
minibuffer-complete-and-exit can't do that, because it doesn't know how
to convert the user string to the absolute file name; as far as it
knows, "foo-file1.text<Alice/alice-1/>" is not a valid completion for
"c:/.../uniquify-file-resources/Alice/alice-1/foo-file1.text", because
they are not string-equal.

Another way to look at this; the minibuffer-level completion functions
only have access to user format strings; ie
"foo-file1.text<Alice/alice-1/>". minibuffer-complete-and-exit could
return that, but we want it to return the absolute file name, as the
substring completion style does, when used on a list of absolute file
names.

>> Together with the advice on completing-read-default and
>> test-completion, this could be moved to minibuffer.el.
>
> As you mention in uniquify-files.el:
>
>     (defun completion-get-data-string (user-string table pred)
>     [...]
>         ;;  FIXME: This is ultimately called from
>         ;;  `completion-try-completion' or `completion-all-completions';
>         ;;  there is only one style currently being used. Need to pass that
>         ;;  style from there to here.
>
> it only makes sense to call the conversion function corresponding to the
> style that was used to generate that string.
>
> [ Also while a specific call to minibuffer-complete (or
>   minibuffer-completion-help, or minibuffer-force-complete, ...) only
>   uses a single style, a given completing-read session can currently use
>   several completion styles.    ]
>
> So I think we should fix this FIXME before we can move this code to
> minibuffer.el.  Maybe we can save the completion style that returned
> that string in a text-property, or even directly store the conversion
> function in there (so we don't need to extend completion-style-alist).

If we move to a completion object storing various things, we can
store it there. Which doesn't address making this work in emacs 26.

Hmm. This FIXME comment lies; completion-get-data-string is called after
completing-read-default, via uniq-file-completing-read-default-advice.
At that point, user-string was computed by the last style tried (ie
uniq-file-all-completions, which could set a text property), but I'm not
sure if it's copied (losing the text properties) in the middle
somewhere. I'll try it.

Similarly for completion-to-table-input.

-- 
-- Stephe