Possible integration with project.el?

[leungbk]

I recall seeing on Bozhidar's blog a year or two ago that he was interested in integrating projectile with Emacs' built-in project.el. Packages like deadgrep (frontend to ripgrep) are using project exclusively, while some other packages prioritize projectile over project in a hard-coded manner, which can lead to some confusion when you're using both types of packages in the same project.

It would be nice to see a single custom agreed on by package authors. If there is interest in making projectile complementary to project, this would entail moving some stuff from projectile to project, and/or reusing some of project's utilities in projectile

[bbatsov]

The only thing I had in mind was adding project.el to the list of project sources in Projectile, and also providing a project.el source based on Projectile. In general I've got no plans to rebase Projectile on top of project.el and plan to continue developing it in an independent fashion.

It would be nice to see a single custom agreed on by package authors. If there is interest in making projectile complementary to project, this would entail moving some stuff from projectile to project, and/or reusing some of project's utilities in projectile

As packages like deadgrep rarely need more than a project root from project.el or projectile I don't think that's that big of a deal. Ideally such packages should check for Projectile and if it's not present use project.el (with the assumption that if some user installed Projectile probably they'd like to use it).

There's not much from project.el that Projectile can reuse, other that project root resolution. If there's something concrete you have in mind, please let me know.

I get that it's more convenient to have a single standard, but I also don't think that the fact there's a built-in Emacs package would mean that alternatives packages (especially those much older than it), should be forced to embrace it. magit vs Emacs's built-in vc package immediately comes to my mind. I'm happy that project.el exists and that it's getting better, but I'm also very happy with the state of Projectile and don't want to waste my limitted time doing changes that won't add any real benefits to the end users.

On the subject of moving code from Projectile to project.el, there's also the restrictive contribution terms of Emacs which require a contributor agreeement that very few of Projectile's contributors have signed. I think this damn agreement has been a major obsticle for Emacs's upstream attracting more contributors and it's the primary reason I prefer to keep my packages outside of GNU ELPA. A lot time ago Stefan Monnier approached me to discuss adding Projectile to Emacs, but I've turned down this kind proposal because I thought that'd limit a lot the number of Projectile contributors.

Btw, you'll also notice that Projectile provides some integration with ripgrep itself. (as it does for other similar packages as well, again for historical reasons)

[bbatsov]

One more thing - I wouldn't say that lsp-mode does anything in a hardcoded manner (https://github.com/emacs-lsp/lsp-mode/blob/fb4c35c6978415c4cf52f85230b527d311989063/lsp-mode.el#L3433-L3436), it does exactly what I suggested. If you installed Projectile probably you'd prefer using Projectile for your project root resolution. Yeah, one can argue that you can have a defcustom to choose explicitly what to use, but I don't think that's such a big deal.

[dgutov]

providing a project.el source based on Projectile

It has been a long time coming. Having projectile at the front of project-find-functions would be much better that forcing everybody to write that long cond like lsp-mode does, don't you think?

In general I've got no plans to rebase Projectile on top of project.el and plan to continue developing it in an independent fashion.

There are a lot of trivial interactive functions in Projectile that now have some counterparts in project.el. You could eliminate that duplication, if you wanted. Or not; that's admittedly of little consequence to end users.

[bbatsov]

@leungbk FWIW - I've updated the docs to show how easy it is to use project-current with Projectile https://docs.projectile.mx/projectile/projects.html#customizing-project-detection

As you can see, project detection itself is a tiny part of the work of such a package and it is easy to change with whatever you want.

[dgutov]

@bbatsov This is the other way around. Which is a fine thing to do by itself, I suppose.

But I don't understand: do you really not care for the idea of project-find-regexp (a command that is present in Emacs keymaps by default now) and xref-find-references (the default impl) using Projectile?

[bbatsov]

@bbatsov This is the other way around. Which is a fine thing to do by itself, I suppose.

It seemed to me like a reasonable approach for someone already using project.el, who wants some extra commands available in Projectile. But yeah - I know the OP had something different in mind. :-)

do you really not care for the idea of project-find-regexp (a command that is present in Emacs keymaps by default now)

I guess that's something like projectile-grep, right?

and xref-find-references (the default impl) using Projectile?

I've been meaning to implement this for a while now, but between all the projects it feel between the cracks. I recall I was looking for some API docs a while back, didn't find any and I forgot about it. I wouldn't mind implementing this in terms of the reference implementation.

There are a lot of trivial interactive functions in Projectile that now have some counterparts in project.el. You could eliminate that duplication, if you wanted. Or not; that's admittedly of little consequence to end users.

That's more or less my take as well. For Projectile to be able to leverage directly some function it will need to contribute its project discovery function to the top of project-find-functions I guess. But in the unlikely scenario of someone using both at the same time this might have weird results (e.g. they might not expect that Projectile changed project.el's find-functions). Anyways, I'll document this approach as well.

[dgutov]

It seemed to me like a reasonable approach for someone already using project.el, who wants some extra commands available in Projectile. But yeah - I know the OP had something different in mind. :-)

I suppose. But there are not many (any?) popular alternative backends for project.el. Some users add their own custom fallback backends, but they can do the same to Projectile, if they wanted, even without this bridge.

I guess that's something like projectile-grep, right?

Something like. But with a different standard UI and a "replace in all matches" command.

It's also "something like" projectile-rg (depending on an upcoming custom option).

I've been meaning to implement this for a while now

I'm not sure it makes much sense to provide an alternative default implementation for the generic, but it can work, I suppose.

But in the unlikely scenario of someone using both at the same time this might have weird results (e.g. they might not expect that Projectile changed project.el's find-functions)

Not sure why you think it's unlikely. project.el's extensibility was designed with this exact scenario in mind:

• The core functions/commands are written against this API, not caring how it is implemented.
• Users keep whatever packages they are accustomed to, including their settings in existing .projectile files.

[dgutov]

Anyways, I'll document this approach as well.

I'd rather it was automatic, but this should be good too. Thanks!

[bbatsov]

Something like. But with a different standard UI and a "replace in all matches" command.

Got it. I'll play with it when I get the chance.

It's also "something like" projectile-rg (depending on an upcoming custom option).

You'd probably want to have there ag and pt. Everyone has a different favourite grep replacement. I've using ag for ages.

I'm not sure it makes much sense to provide an alternative default implementation for the generic, but it can work, I suppose.

Can you share a link to the default implementation? I'm wondering how exactly are the usages discovered, as I guess if it's not generic you're not really parsing all the code in the project.

The core functions/commands are written against this API, not caring how it is implemented.

Same with Projectile. The API is basically 5 functions and everything else is derived from it.

The core functions/commands are written against this API, not caring how it is implemented.

Given that Projectile and project.el work pretty much the same in this regard - run a list of functions until they get a result, I'm pretty sure the order of the functions can affect the results that users get. :-)

I'd rather it was automatic, but this should be good too. Thanks!

Sure, if that's fine by you I'll just have Projectile register its project function automatically.

[dgutov]

You'd probably want to have there ag and pt.

Should be easy to add them, too. As long as they support the necessary flags.

Can you share a link to the default implementation?

Sure: https://github.com/emacs-mirror/emacs/blob/4b25ffd3b8ffb854c2458efc4914829ead9a0f79/lisp/progmodes/xref.el#L259-L276

I'm wondering how exactly are the usages discovered, as I guess if it's not generic you're not really parsing all the code in the project.

Of course. It's a fallback implementation.

Same with Projectile. The API is basically 5 functions and everything else is derived from it.

The core can't reference Projectile's functions directly. 🤷‍♂️

I'm pretty sure the order of the functions can affect the results that users get.

In practice only 1-2 backends will be used on a regular basis. So as long as Projectile is the first item in the list, it should take priority. And as a minor mode, it can be there first.

Anyway, that's not the implementation detail I was talking about.

Sure, if that's fine by you I'll just have Projectile register its project function automatically.

👍

Maybe also add an option for a rare user who'd prefer to opt out or do the integration manually.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

[cyruseuros]

Here's a working implementation. It could be a tad faster if projectile and project.el both used either absolute or relative paths. As it stands we need to do some conversion. I also considered using mapcan and projectile-files-in-project-directory but it added one more branch point that didn't add anything in terms of performance

(require 'cl-lib)

(cl-defmethod project-root ((project (head projectile)))
  (cdr project))

(cl-defmethod project-files ((project (head projectile)) &optional dirs)
  (let* ((root (project-root project))
         ;; make paths absolute
         (files (mapcar (lambda (f)
                          (expand-file-name f root))
                        (projectile-project-files root))))
    ;; filter out files that don't have the same prefix
    (if dirs
        (cl-remove-if-not
         (lambda (f)
           (cl-some (lambda (dir)
                      (string-prefix-p dir f))
                    dirs))
         files)
      files)))

(defun project-projectile (dir)
  "Return projectile project of form ('projectile . ROOT-DIR) for DIR"
  (let ((root (projectile-project-root dir)))
    (when root
      (cons 'projectile root))))

(provide 'project-projectile)

All you need to do after is

(add-hook 'project-find-functions
            #'project-projectile
            'append)

[dgutov]

Good effort. 👍 A few notes:

• The dirs argument in project-files can probably be ignored, since Projectile has no support for "external roots" of any kind. So the only dir the method can be called with, is the project root. If that were not the case, you could compare dirs with the root (like, equal or not). That would be faster than filtering the whole list (dirs are not supposed to be subdirectories, thus far). But anyway, you can just ignore the argument.
• The last argument to add-hook (the last form) should probably be omitted: if the user has installed Projectile and enabled it, they probably want it to take over the project-finding and indexing role, rather than come last in the list.
• The relative-absolute conversion is unfortunate, but project.el will have absolute file names, so it seems prudent to standardize on that format. No suggestion there, sorry.

[cyruseuros]

Thanks for the feedback @dgutov! I didn't know external roots couldn't be subdirs though it makes perfect sense in hindsight. As for no external roots in projectile I guess that explains why the project can be represented as its root alone.

Would something like this be welcome as part of projectile then?

(require 'cl-lib)

(cl-defmethod project-root ((project (head projectile)))
  (cdr project))

(cl-defmethod project-files ((project (head projectile)) &optional dirs)
  (let ((root (project-root project)))
    ;; make paths absolute and ignore the optional dirs argument,
    ;; see https://github.com/bbatsov/projectile/issues/1591#issuecomment-896423965
    (mapcar (lambda (f)
              (expand-file-name f root))
            (projectile-project-files root))))

(defun project-projectile (dir)
  "Return projectile project of form ('projectile . ROOT-DIR) for DIR"
  (let ((root (projectile-project-root dir)))
    (when root
      (cons 'projectile root))))

(provide 'project-projectile)

(add-hook 'project-find-functions
          #'project-projectile)

[dgutov]

@wurosh For better performance, you might also prefer concat over expand-file-name:

(cl-defmethod project-files ((project (head projectile)) &optional dirs)
  (let ((root (project-root project)))
    ;; make paths absolute and ignore the optional dirs argument,
    ;; see https://github.com/bbatsov/projectile/issues/1591#issuecomment-896423965
    (mapcar (lambda (f)
              (concat root f))
            (projectile-project-files root))))

[dgutov]

And now the ball is in @bbatsov's court.

[vspinu]

Hey! Trying to keep this one alive.

It would be indeed a very welcoming addition. A great simplification to thousands of devs out there who would not need to integrate support for both packages.

[bbatsov]

I totally forgot about this ticket, but I'll (finally) add this patch to Projectile soon.