Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow specifying project name in the dirconfig file #1801

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Thuna-Cing
Copy link

This allows specifying the project name in the .projectile file.

It is backwards compatible as the older versions will parse the name as a comment.

All tests except one that also fails on origin are passing so I marked them as such.
There should be a new test added to actually check that the project name is being
parsed properly but it can wait until the rest of the change is finalized, similarly for the
readme and the changelog.


Before submitting a PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):

  • The commits are consistent with our contribution guidelines
  • You've added tests (if possible) to cover your change(s)
  • All tests are passing (eldev test)
  • The new code is not generating bytecode or M-x checkdoc warnings
  • You've updated the changelog (if adding/changing user-visible functionality)
  • You've updated the readme (if adding/changing user-visible functionality)

* projectile.el (projectile-dirconfig-name-prefix): Add option.
(projectile-project-default-project-name): Use the project name in
dirconfig when given, use the directory name otherwise.
(projectile-parse-dirconfig-file): Parse and return the project name
as the fourth element.
* test/projectile-test.el (projectile-parse-dirconfig-file): Expect
the fourth element representing the project name.

This is just to make sure the tests succeed.  A proper test for
project names should be written.
@bbatsov
Copy link
Owner

bbatsov commented Oct 25, 2022

Isn't it much easier to set a custom project name using .dir-locals.el?

@Thuna-Cing
Copy link
Author

Thuna-Cing commented Oct 25, 2022

Isn't it much easier to set a custom project name using .dir-locals.el?

.dir-locals.el doesn't work reliably in non-file buffers. (Also, it is just more convenient in .projectile in my opinion)

@bbatsov
Copy link
Owner

bbatsov commented Oct 25, 2022

But at the cost of adding more complexity to an already complex thing. That's why I'm a bit on the fence about this. I wonder if it won't be simpler to introduce a new file like .projectile-info or something where additional project data can be stored aside from ignore/include stuff.

@Thuna-Cing
Copy link
Author

Thuna-Cing commented Oct 25, 2022

Does projectile use (or intend to use) any information other than the project name? If it will also contain the projectile configuration in .dir-locals.el, then it might be worth it to create a different .projectile-info file but it seems overkill otherwise.

Besides, if we have so much information that a different projectile-specific file is necessary, I would lean more toward making .projectile a directory instead.

@bbatsov
Copy link
Owner

bbatsov commented Oct 25, 2022

At some point I was thinking of making some more structured configuration file to replace .projectile, but I put this idea on the backburner as with .dir-locals.el we've managed to address 90+% of what was needed essentially for free. And it solves the most complex problem - keep in sync the configuration file and the runtime configuration.

projectile.el Outdated
@@ -1299,7 +1308,9 @@ explicitly."
(defun projectile-default-project-name (project-root)
"Default function used to create the project name.
The project name is based on the value of PROJECT-ROOT."
(file-name-nondirectory (directory-file-name project-root)))
(or (let ((default-directory project-root))
(nth 3 (projectile-parse-dirconfig-file)))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the project name is constantly recalculated in the modeline parsing the config file all the time can make things extremely slow. That's one more reason I'm wary about the proposed change.

* projecile.el (projectile-default-project-name): Search only for the
name instead of calling `projectile-parse-dirconfig-file' everytime.
@Thuna-Cing
Copy link
Author

Hmm, I didn't realize just how often projectile-project-name was called. Then, instead of projectile-parse-dirconfig-file, a function that only checks for the name should be fast enough. I just made a new commit as a proof of concept, maybe that will be better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants