Re: find-file-project

[Stephen Leake]

Dmitry Gutov <address@hidden> writes:

> We've discussed this before: I think this approach adds too much
> infrastructure (like forces through bits of "flat" project support,
> which aren't optimal for most of the projects I know of).

Only code that needs flat paths uses them. As we discussed earlier, we
can either compute the flat path, and do completion on it, or compute
the flat path while doing completion.

And as we also discussed earlier, for _some_ projects, flat paths _are_
optimal.

What is the downside of a few dozen lines of code to support a few
projects? 

> Why not just implement completion on file paths relative to the
> project root? 

For Emacs elisp, there is no single root, except perhaps "/".

> The user could input a base file name, if they like, and TAB would
> expand it to one of the relative paths if it's unique, or allow them
> to input a directory. You won't need any other uniquification then.

That requires the user to know what directory the file is in, as
find-file completion does now.

Using a flat path avoids that. I find it quite useful to just type
"locate", and immediately see that there are two choices, one of which I
was unaware of.

On the other hand, what you are describing is pretty much what
find-file-project does. Have you tried it?

> Maybe call it find-file-in-project?

That does sound better.

>> The patch also adds small projects for elisp and global, to show that
>> this approach works for multiple backends.
>
> I don't see the elisp backend.

Oops; that got left out of the patch. This code is supposed to be in
project.el:
    
(cl-defstruct project-elisp
  search-path-flat
  ;; list of non-recursive directory names.
)

(defun project-elisp-make (extra-load-path)
  "Return a `project-elisp' object initialized from EXTRA-LOAD-PATH and 
`load-path'.
For example, add \"lisp/cedet/ede\" for core emacs."
  (make-project-elisp
    :search-path-flat (append extra-load-path load-path)))

(cl-defmethod project-flat-search-path ((prj project-elisp))
  "Override `project-flat-search-path' for elisp projects."
  (project-elisp-search-path-flat prj))

(cl-defmethod project-search-path ((_prj project-elisp))
  "Override `project-search-path' for elisp projects."
  load-path)

(cl-defmethod project-root ((_this project-elisp))
  ;; no meaningful root
  nil)

(cl-defmethod project-identifier-completion-table ((_this project-elisp))
  (elisp--xref-identifier-completion-table))

(cl-defmethod project-ignore-files-globs ((_this project-elisp))
  '("*.elc"))

;; example project; used for emacs

(defvar project-emacs
  (let ((cedet-root (file-name-directory (locate-file "cedet.el" load-path))))
    (project-elisp-make
     (project-recursive-ignores-to-flat
      (list
       (concat cedet-root "ede")
       (concat cedet-root "semantic")
       (concat cedet-root "srecode"))
      nil)
     )))

(defun project-try-elisp-emacs (_dir)
  (when (memq major-mode '(emacs-lisp-mode lisp-interaction-mode))
    project-emacs))


> Regarding global, is there no related semantic-symref-tool command?
> Maybe it would make sense to implement this in a generic fashion.

I've also implemented ede-find-file, which dispatches to the
semantic-symref global backend. Next patch.

>> I can break this into smaller commits on master, if that seems like a
> good idea.
>
> If you want to test it out, please commit it to a feature branch
> instead.

I've already tested it. I guess if you mean if I want others to test it.
I was hoping this patch would be enough.

>> +(defun find-file-complete-global (filename)
>> +  "Prompt for completion of FILENAME in a Gnu global project."
>> +    (setq filename
>> +      (completing-read
>> +       "file: " ;; prompt
>> +       (completion-table-with-cache #'find-file-complete-global-table) ;; 
>> collection
>> +       nil ;; predicate
>> +       t ;; require match
>> +       filename
>> +       ))
>
> I don't think each implementation should do its own completing-read.
> Rather, they should just return the completion table from a generic
> method. E.g. (cl-defmethod project-file-completion-table ...).

For the default find-file-in-project, that would require computing the
flat path and the predicate on each use of the completion table. Other
instances might want to bind completion-ignore-case or
completion-regexp-list.

More importantly, the result of the completion is treated differently; the
default instance calls locate-file after rearranging the uniquification,
while the global instance calls ceded-gnu-global-call.

>> +    ;; and the user completes to the first, the following global call
>> +    ;; will return both. The desired result is always the shortest.
>
> I'd say the better match wins (not the shortest path), but if you want
> to add sorting, completion-metadata can include display-sort-function.

I don't follow; the completion does not need to be sorted. This code is
dealing with the result of completion.

A better way to accomplish this would be to somehow encode the full
directory path in the completion result, but I didn't find a way to do
that. In particular, text properties are not returned from completion.

>> +;;; project.el integration
>> +
>> +(defun project-try-global (dir)
>> +  (when (cedet-gnu-global-version-check t)
>> +    (let ((root (locate-dominating-file dir "GTAGS")))
>> +      (when root
>> +    (list 'global root)))))
>> +
>> +(cl-defmethod project-find-file ((prj (head global)) filename)
>> +  (let ((default-directory (file-name-as-directory (nth 1 prj))))
>> +    (find-file (find-file-complete-global filename))))
>
> ...if you're sure it's a good idea. After all, GNU Global is just a
> tool, it doesn't (and cannot) know too much about a project.
>
> If it's used as just-another project implementation, you won't be able
> to integrate it with some more advanced kind of project definition.

It's on a par with the existing EDE "generic" projects for vc tools. And
with the existing project-vc.

>> +(defconst find-file-uniquify-regexp "^\\(.*\\)<\\(.*\\)>"
>> +  "Regexp matching uniqufied file name.
>> +Match 1 is the filename, match 2 is the relative directory.")
>
> This is way complicated. completion-at-point interface will help with
> typing, no real need to shorten the file names.

I don't follow; what file names are being shortened? On the contrary,
they are being lengthened, with enough directory names to make them unique.

> Another benefit of delegating all that to a completion table, is that
> different kinds of frontends would be able to use it.

This code is used in the completion table, and can be used in any
completion table that has similarly colliding names.

For example, if the user enters "locate" <tab> at the find-file-project
prompt, the display then shows:

file: locate(.){c, rnc, el<lisp>, el<lisp/cedet/ede>}

showing that there are four possible completions, two with the same
filename. 

>> +;; Conversion between recursive and flat paths.
>> +(defun project--directory-directories-1 (dir ignore-regexp)
>> ...
>> +(defun project--directory-directories-recurse (dir ignore-regexp)
>> ...
>> +(defun project-recursive-ignores-to-flat (recursive-path ignore-dirs)
>> ...
>> +(defun project-flat-to-recursive-ignores (flat-path)
>> ...
>> +(cl-defgeneric project-ignore-dirs (_prj)
>> ...
>> +(cl-defgeneric project-flat-search-path (prj)
>> ...
>
> Do you really need all this?

Yes.

I guess you are asking for some rationale. This code allows converting
between the two equivalent representations of search paths;
recursive/ignores and flat. As we have discussed before, there are cases
where both of these conversions are useful.

>> +  ;; FIXME: do we need both project-ignores and project-ignore-files-globs?
>
> Just having project-ignores should suffice.

I guess I could just ignore 'dir' in project-ignores. 

-- 
-- Stephe