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

Smarter projectile-edit-dir-locals #1135

Open
basil-conto opened this issue Apr 23, 2017 · 10 comments
Open

Smarter projectile-edit-dir-locals #1135

basil-conto opened this issue Apr 23, 2017 · 10 comments
Labels
Good First Issue Help Wanted Improvement Pinned Tickets that are immune to the staleness checks

Comments

@basil-conto
Copy link
Contributor

Expected behaviour

More control over location of and workflow around dir-locals-files.

In this issue I wish to:

  1. Draw attention to some of the more obvious limitations of the command projectile-edit-dir-locals.
  2. Clarify in what way and to what extent Projectile aims to support per-directory local variables.

Actual behaviour

Limitations of command projectile-edit-dir-locals:

  • The dir-locals-file name is currently hard-coded as ".dir-locals.el", which does not take dir-locals-file-2 or MS-DOS limitations into account.

  • The dir-locals-file is expanded relative to (projectile-project-root) and thus does not support nested dir-locals-files.

Steps to reproduce the problem

$ cd
$ emacs --quick
;; Create project root
C-x C-f test-project/.projectile
C-o C-x C-s y

;; Create root `dir-locals-file'
C-x C-f .dir-locals.el
C-o C-x C-s

;; Create leaf `dir-locals-file'
C-x C-f lisp/.dir-locals.el
C-o C-x C-s y

;; Create project source file
C-x C-f foo.el
C-o C-x C-s
$ tree -a ~/test-project/
test-project/
├── .dir-locals.el
├── lisp
│   ├── .dir-locals.el
│   └── foo.el
└── .projectile

1 directory, 4 files
;; Edit `dir-locals-file' via Projectile
M-x package-initialize
M-x projectile-edit-dir-locals

;; Check current file
M-: (abbreviate-file-name (buffer-file-name)) ; => "~/test-project/.dir-locals.el"

Environment & Version information

Projectile version information

ELISP> (projectile-version)
"20170416.148"
$ cd /path/to/projectile/repo/
$ git describe --abbrev=0 --tags # latest tag
v0.14.0

Emacs version

ELISP> (emacs-version)
"GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2017-03-26"

Operating system

$ uname --kernel-name --kernel-release --machine --operating-system
Linux 4.9.0-2-amd64 x86_64 GNU/Linux
$ cat /etc/debian_version 
9.0

Request for Comments

The following suggestions address some of the aforementioned limitations.

  • Projectile could first try to find the nearest dir-locals-file by default, e.g.
;; Note: this does not stop at (projectile-project-root)
(let ((locals (dir-locals-find-file default-directory)))
  (when locals
    (find-file (car (last (dir-locals--all-files
                           (or (car-safe locals) locals)))))))
  • The user could be offered a list of all dir-locals-files found between default-directory and (projectile-project-root).

  • A customisation variable could determine whether to prefer the root vs nearest dir-locals-file found, whether to ask the user or whether to offer possible completions.

  • If no dir-locals-files are found, the user could be prompted for a directory in which to create one.

I would appreciate feedback on the given issues and suggestions. I would also be happy to work on a patch for any ensuing decisions, if I am given some initial pointers.

Thank you for this excellent and useful package!

@basil-conto
Copy link
Contributor Author

[projectile-edit-dir-locals] does not take MS-DOS limitations into account

Actually, expand-file-name could be doing the necessary dosification of the filename ".dir-locals.el", but I am unable to test this. Either way, I think it may be wiser to delegate as many file operations as possible to built-in functions.

@bbatsov
Copy link
Owner

bbatsov commented Sep 29, 2018

PRs welcome! :-)

@stale
Copy link

stale bot commented May 8, 2019

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!

@stale stale bot added the Stale label May 8, 2019
@basil-conto
Copy link
Contributor Author

Testing whether this comment counts as activity. ;)

@stale stale bot removed the Stale label May 8, 2019
@stale
Copy link

stale bot commented Aug 7, 2019

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!

@stale stale bot added the Stale label Aug 7, 2019
@basil-conto
Copy link
Contributor Author

Ping.

@stale stale bot removed the Stale label Aug 7, 2019
@stale
Copy link

stale bot commented Feb 3, 2020

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!

@stale stale bot added the Stale label Feb 3, 2020
@basil-conto
Copy link
Contributor Author

Ping.

@stale stale bot removed the Stale label Feb 3, 2020
@bbatsov bbatsov added Good First Issue Pinned Tickets that are immune to the staleness checks labels Feb 4, 2020
@bbatsov
Copy link
Owner

bbatsov commented Feb 4, 2020

@basil-conto It'd be even better if you tried to implement some of your ideas. Unfortunately I'm short on time for Projectile these days and this issue is fairly low in my todo list.

@basil-conto
Copy link
Contributor Author

Same here, and I've since switched from using Projectile to the built-in project.el. I'll try looking into this later today or over the coming weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue Help Wanted Improvement Pinned Tickets that are immune to the staleness checks
Projects
None yet
Development

No branches or pull requests

2 participants