Re: [Emacs-diffs] scratch/project 106e023: Add new `project' package, and use it in xref

Re: [Emacs-diffs] scratch/project 106e023: Add new `project' package, and use it in xref

[Stefan Monnier]

> +(defvar project-functions '(project-try-vc
> +                            project-try-ede
> +                            project-ask-user)

I think I'd prefer a slightly more verbose name for this variable.
Also, I'd suggest you use (list #'foo ... #'bar) so that we
automatically check that those functions are known.

> +(cl-defgeneric project-root (project)
> +  "Return the root directory of the current project.")

This should specify that it's an absolute file name.

> +(cl-defgeneric project-source-directories (project)
> +  "Return the list of source directories.
> +Including any where source (or header, etc) files used by the
> +current project may be found.  Including those outside of the
> +project tree."
> +  (project-directories project))

Same here: we should explicitly say if relative file names can appear or
not in the output.

> +(defvar project-vc-root-files '(".git" ".hg" ".bzr"))

I'd appreciate if this could be better integrated with VC (so it
automatically picks up other names for other supported backends).

> +(defun project-try-ede (dir)
> +  (when (featurep 'ede)
> +    (cons 'ede (ede-directory-get-open-project dir 'ROOT))))

Why use a cons cell with the car's symbol standing for a type tag, when
the carried object is already properly self-describing (IIUC it's an
EIEIO object)?


        Stefan

Re: [Emacs-diffs] scratch/project 106e023: Add new `project' package, and use it in xref

[Dmitry Gutov]

On 07/08/2015 06:44 AM, Stefan Monnier wrote:

> I think I'd prefer a slightly more verbose name for this variable.
> Also, I'd suggest you use (list #'foo ... #'bar) so that we
> automatically check that those functions are known.

Sounds good. Please take a look again.

> Same here: we should explicitly say if relative file names can appear or
> not in the output.

Done.

>> +(defvar project-vc-root-files '(".git" ".hg" ".bzr"))
>
> I'd appreciate if this could be better integrated with VC (so it
> automatically picks up other names for other supported backends).

Also done.

>> +(defun project-try-ede (dir)
>> +  (when (featurep 'ede)
>> +    (cons 'ede (ede-directory-get-open-project dir 'ROOT))))
>
> Why use a cons cell with the car's symbol standing for a type tag, when
> the carried object is already properly self-describing (IIUC it's an
> EIEIO object)?

Because then dispatching any `project-root' call fails at runtime if ede 
is not loaded. And I don't want to make it a runtime dependency.

This function should probably be in ede.el, and only added to 
project-find-functions when it's loaded.

Any comments on the "source directories" issue? I'm inclining towards 
having a project-source-directories-function variable now, which in 
emacs-lisp-mode would return load-path.

This is also complicated by `project-find-functions' being dependent on 
the directory, but emacs-lisp-mode can only set a buffer-local value, in 
its buffers.

Re: [Emacs-diffs] scratch/project 106e023: Add new `project' package, and use it in xref

[Stefan Monnier]

> Sounds good. Please take a look again.

Good, thanks.

>> Why use a cons cell with the car's symbol standing for a type tag, when
>> the carried object is already properly self-describing (IIUC it's an
>> EIEIO object)?
> Because then dispatching any `project-root' call fails at runtime if ede is
> not loaded. And I don't want to make it a runtime dependency.
> This function should probably be in ede.el, and only added to
> project-find-functions when it's loaded.

Exactly: rather than work around the problem by using a cons cell,
better do it by moving the function where it belongs.

> Any comments on the "source directories" issue? I'm inclining towards
> having a project-source-directories-function variable now, which in
> emacs-lisp-mode would return load-path.

I haven't had time to look into that yet, sorry.


        Stefan

Re: [Emacs-diffs] scratch/project 106e023: Add new `project' package, and use it in xref

[Dmitry Gutov]

On 07/08/2015 05:06 PM, Stefan Monnier wrote:

> Exactly: rather than work around the problem by using a cons cell,
> better do it by moving the function where it belongs.

Done.

>> Any comments on the "source directories" issue? I'm inclining towards
>> having a project-source-directories-function variable now, which in
>> emacs-lisp-mode would return load-path.
>
> I haven't had time to look into that yet, sorry.

I'm fairly content with the latest iteration, but please take a look 
when you have the time anyway.