On 2017-07-23 16:31, Eli Zaretskii wrote:>> This part is wrong, because locate-dominating-file also accepts directories:
>>
>>   Look up the directory hierarchy from FILE
> 
> Actually, FILE _must_ be a directory, because the function does this:
> 
>       (setq try (if (stringp name)
>                     (file-exists-p (expand-file-name name file))
>                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^

Ouch.  This is a problem, because I'm not the only one who assumed that this had to be a file and not a directory.  There are instances of locate-dominating-file being called with file-name in vc, trampver, yasnippet, company, flycheck, proof-general, etc.  Github finds 35k matches for (locate-dominating-file buffer-file-name) (https://github.com/search?q=%28locate-dominating-file+buffer-file-name&type=Code&utf8=%E2%9C%93).

> It's possible that "directory hierarchy from FILE" doesn't convey that
> clearly enough, in which case we could add
> 
>   FILE should be a directory.

Yes, this would be great.  In fact, I think we should rename the argument to DIRECTORY, if it's a directory.
But I'm not sure what we should do about all the existing callers…

>> This part is wrong, because the predicate is called with the initial file name, too:
>>
>>   NAME can also be a predicate taking one argument (a directory)
> 
> Why you say that this is wrong?  The doc string never said anything to
> the contrary.

The docstring says "one argument (a directory)", but locate-dominating-file (when called with a file name, not a directory name) passes that file name to NAME (thus calling it with one argument that's not a directory).

I understand now that this isn't a valid use of locate-dominating-file, however, so that point is moot.  I was under the impression from the docstring that locate-dominating-file had to be called with a file name, not a directory name.

Thanks for the explanations!
Clément.