emacs-python-pytest icon indicating copy to clipboard operation
emacs-python-pytest copied to clipboard

Add support for using project.el instead of projectile

Open alexmurray opened this issue 2 years ago • 15 comments

project is shipped in Emacs and in ELPA and should be able to support most of the functionality that projectile does - could this be supported as an alternative?

alexmurray avatar Dec 15 '21 01:12 alexmurray

tl;dr: tempted to close as wontfix

🤔 what would be the benefits?

it seems project.el is experimental and from a quick look it lacks various projectile features that are used by this project, such as for finding and filtering test files, useful file sorting, the ‘save buffers?’ prompt, etc.

and even if that was not the case, i am not looking forward to maintaining almost-duplicate code paths using building blocks that will undoubtedly be different in various subtle ways.

while projectile is currently a hard requirement, it is used in a rather non-intrusive way (only under the hood), and should generally not even be visible to users?

wbolster avatar Dec 15 '21 10:12 wbolster

+1 for removing projectile from dependencies. I'm using project.el.

ATM the python-pytest-function-dwim doesn't work for me. Just the message:

projectile--test-name-for-impl-name: Project type ‘make’ not supported!

muffinmad avatar Feb 04 '22 15:02 muffinmad

that error seems unrelated to this project. somehow projectile doesn't recognize your project as a python project

wbolster avatar Feb 04 '22 17:02 wbolster

re project.el support: a technical analysis of how all the protectile features used by this package would map to (or are missing from) project.el would be helpful to properly assess feasibility.

there are not too many but some of them are relatively complex such as corresponding test file discovery.

to be clear: for me feasibility also explicitly includes maintainability, e.g. not eager to add a ton of code

wbolster avatar Feb 04 '22 17:02 wbolster

that error seems unrelated to this project. somehow projectile doesn't recognize your project as a python project

Indeed. And now I have to deal with projectile. But I don't want to :)

there are not too many but some of them are relatively complex such as corresponding test file discovery.

How about to not rely on projectile functions themselves but provide customizable options for all those functions? E.g.

(defcustom python-pytest-project-root-function #'projectile-project-root
  "Function to return project root."
  :type 'function)

In case I do not want to use projectile, I can specify different function to get the project root instead.

Just thinking out loud :)

Anyway, thanks for your great job! I will surely use this package. Even with projectile :)

Thanks!

muffinmad avatar Feb 04 '22 18:02 muffinmad

yeah something like that could work but i don't want it on the function level because i want to keep it an implementation detail instead of a published interface. i would rather have a single 'project-backend' setting or similar. if possible at all without too much pain.

finding the project root is the simplest example. it's the other stuff that worries me. as i said, a technical comparison of what this package currently consumes from projectile vs. (potentially missing) project.el counterparts would be welcome

wbolster avatar Feb 04 '22 18:02 wbolster

From what I can see:

(projectile-project-name) -> (project-name (project-current t)) (projectile-compilation-dir) -> (project-root (project-current t) (project supports subprojects if a marker exists) (projectile-test-file-p <file>) -> Would need to be added in project.el (projectile-find-matching-test) -> Missing but maybe could be done using find-sibling-file (projectile-project-files) -> (project-files (project-current t)) (projectile-sort-by-recentf-first) -> This is just a mapcar and could live in this project (projectile-buffers) -> (project-buffers (project-current t)) (projectile-buffers-with-file) -> This is also just a cl filter.

Projectile messes with project.el (or vc?) in strange ways which is why I'd also support making it optional and one of two possible providers. But I also can see how project.el is still not quite as complete (and settled).

Walheimat avatar Jan 17 '23 18:01 Walheimat

Just ran into this myself: projectile adds project-projectile to project-find-functions, which causes (project-current) to return something like (projectile . "/path/to/current/project"). That in turn causes problems for project.el, because it doesn't recognise it as a project. So it thinks you're not in a project buffer, even if you are.

I'm not sure why projectile does this, to be honest, it doesn't really make sense to me.

If I'm honest, though, I think a package such as python-pytest shouldn't have a hard dependency on other 3rd-party packages that aren't libraries. projectile.el is a user-facing package and can (and indeed does) cause problems for users that don't already use it.

Ideally, python-pytest would use either project.el or projectile.el, depending on what the user is using. @wbolster I can try and put a PR together, if you would be willing to consider including it.

joostkremers avatar Mar 13 '23 12:03 joostkremers

aside: note that i don't care about either the ‘waste of space’ or puristic ‘gnu/zealotism’ arguments. (update: made it more clear this remark is not a reply but an aside)

projectile-as-a-library worked quite well so far, even though it's (also) a user facing package. so far it has not resulted in any practical problems. but now it seems that

projectile adds project-projectile to project-find-functions

… which used to not be the case at all. looks like this line:

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

was added not too long ago via https://github.com/bbatsov/projectile/issues/1591. it has some discussion about project-find-functions specifically (and i haven't bothered to read that yet).

the various projectile.el features related to listing and filtering project files seem to have a project.el counterpart; see this comment, but two features that power some very useful python-pytest.el features apparently don't:

  • projectile-test-file-p
  • projectile-find-matching-test

in fact, things like this were the original reason to rely on projectile.el in the first place, because i prefer to avoid reinventing tangentially related wheels (like project listing, pdb integration, etc) in a package like python-pytest.el.

that said, i general i don't like having multiple code paths for doing essentially the same things unless absolutely necessary. i don't really like adding wrappers around everything used from projectile and adding if/else constructs for projectile vs project.el. maybe eioio methods can make this simpler under the hood by having one interface with 2 implementations, but the main point of having 2 implementations for unclear benefits still stands.

wbolster avatar Mar 13 '23 14:03 wbolster

note that i don't care about either the ‘waste of space’ or puristic ‘gnu/zealotism’ arguments.

I wasn't aware I was doing either...

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

was added not too long ago

Yes, that's a recent addition. I didn't read the entire discussion close enough to really understand why it was added, but this remark:

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 [emphasis mine]).

from bbatsov here seems significant, because the highlighted part is no longer true when Projectile is used as a library.

I understand your point about not wanting to implement functionality that Projectile already provides, though. Which makes me think that the best solution would be to make sure Projectile can be used as a library, i.e., without having any effect on a user's configuration unless explicitly configured by the user.

I'll try and dig a little deeper into the purpose of project-projectile to make sure it's really the source of the problem I've been seeing, and then maybe take up the issue with Projectile's author.

joostkremers avatar Mar 13 '23 19:03 joostkremers

note that i don't care about either the ‘waste of space’ or puristic ‘gnu/zealotism’ arguments.

I wasn't aware I was doing either...

sorry, didn't mean to imply you did.

but i've seen this tendency in the emacs world (melpa, project.el, magit, flymake, etc) and i will not be a part of that 🙃

[…] the best solution would be to make sure Projectile can be used as a library, i.e., without having any effect on a user's configuration unless explicitly configured by the user.

agreed, and i am totally willing to adapt this package if changes are needed to achieve that. please let me know if you find out how that's achieved.

i just don't want to cripple (part of) this package for unclear reasons, especially as that comes with real cost in terms of maintenance, documentation, support, etc.

wbolster avatar Mar 14 '23 07:03 wbolster

but i've seen this tendency in the emacs world (melpa, project.el, magit, flymake, etc) and i will not be a part of that upside_down_face

Oh, don't worry, I won't make such arguments. :slightly_smiling_face:

[…] the best solution would be to make sure Projectile can be used as a library, i.e., without having any effect on a user's configuration unless explicitly configured by the user.

agreed, and i am totally willing to adapt this package if changes are needed to achieve that. please let me know if you find out how that's achieved.

bbatsov made a change so that project-projectile is only added to project-find-functions when the user explicitly enables projectile-mode, not when it's loaded. That solves the problem I encountered without you needing to make any changes to python-pytest.

joostkremers avatar Mar 15 '23 23:03 joostkremers

Just double-checking: does anyone have a (potentially "private") PR that implements project.el support?

pmiddend avatar Nov 23 '23 08:11 pmiddend

@pmiddend I've switched to project-compile for now. When called with prefix argument (C-u C-x p c), it allows to specify the command with arguments to run.

muffinmad avatar Nov 23 '23 08:11 muffinmad