Merge pull request #1995 from bbatsov/dirconfig-polish

Polish: align dirconfig warnings, robust cache key, small cleanups

2 files changed, 75 insertions, 29 deletions

diff --git a/projectile.el b/projectile.el
index f600597..cd3ea2a 100644
--- a/projectile.el
+++ b/projectile.el
@@ -389,6 +389,13 @@ algorithm."
 (defcustom projectile-dirconfig-file
   ".projectile"
   "The file which serves both as a project marker and configuration file.
+
+The mere presence of this file in a directory marks that directory
+as a Projectile project root, even when the file is empty.  When
+the file has content, it is parsed by `projectile-parse-dirconfig-file'
+to drive `+' keep / `-' ignore / `!' ensure rules; see the manual
+for the full format.
+
 This should _not_ be set via .dir-locals.el."
   :group 'projectile
   :type 'file
@@ -673,7 +680,10 @@ project."
 
 (defvar projectile--dirconfig-cache (make-hash-table :test 'equal)
   "Cache for parsed dirconfig files, keyed by project root.
-Each value is a cons of (MTIME . PARSED-RESULT).")
+Each value is a list of (DIRCONFIG-PATH MTIME PARSED-RESULT); a
+cache hit requires both DIRCONFIG-PATH and MTIME to match the
+current file, so changing `projectile-dirconfig-file' mid-session
+naturally invalidates the entry.")
 
 (defvar projectile--alien-dirconfig-warned-projects (make-hash-table :test 'equal)
   "Set of project roots already warned about alien indexing skipping the dirconfig.")
@@ -681,6 +691,9 @@ Each value is a cons of (MTIME . PARSED-RESULT).")
 (defvar projectile--prefixless-dirconfig-warned-projects (make-hash-table :test 'equal)
   "Set of project roots already warned about prefix-less dirconfig entries.")
 
+(defvar projectile--glob-keep-warned-projects (make-hash-table :test 'equal)
+  "Set of project roots already warned about glob patterns in + keep entries.")
+
 (defvar projectile-known-projects nil
   "List of locations where we have previously seen projects.
 The list of projects is ordered by the time they have been accessed.
@@ -1529,11 +1542,14 @@ If PROJECT is not specified acts on the current project."
 
 ;;; Project indexing
 (defun projectile-get-project-directories (project-dir)
-  "Get the list of PROJECT-DIR directories that are of interest to the user."
+  "Get the list of PROJECT-DIR directories that are of interest to the user.
+When the dirconfig file has no `+' keep entries, return a single-
+element list with PROJECT-DIR itself."
   (let* ((cfg (projectile-parse-dirconfig-file))
          (keep (and cfg (projectile-dirconfig-keep cfg))))
-    (mapcar (lambda (subdir) (concat project-dir subdir))
-            (or keep '("")))))
+    (if keep
+        (mapcar (lambda (subdir) (concat project-dir subdir)) keep)
+      (list project-dir))))
 
 (defun projectile--directory-p (directory)
   "Checks if DIRECTORY is a string designating a valid directory."
@@ -2213,17 +2229,27 @@ are accepted for backward compatibility but recorded separately so
 callers can flag the deprecated syntax.  All slots default to nil."
   (keep nil) (ignore nil) (ensure nil) (prefixless-ignore nil))
 
-(defun projectile--warn-glob-in-keep-entry (entry dirconfig)
-  "Warn that ENTRY in DIRCONFIG looks like a glob pattern after a `+'.
-The `+' prefix is for subdirectories only; the parser silently coerces
-each entry to a directory, so a glob pattern would never match."
-  (display-warning
-   'projectile
-   (format "%s contains `+%s', but `+' entries are treated as \
-subdirectory paths and globs are not expanded.  Use a plain directory \
-or move the pattern to a `-'/`!' rule."
-           dirconfig entry)
-   :warning))
+(defun projectile--maybe-warn-glob-keep-entries (project-root cfg)
+  "Warn once per session about glob patterns in + keep entries.
+PROJECT-ROOT identifies the warned-projects set; CFG is the parsed
+`projectile-dirconfig' struct.  The `+' prefix is for subdirectories
+only; the parser silently coerces each entry to a directory, so a
+glob pattern would never match."
+  (when (and cfg
+             (not (gethash project-root projectile--glob-keep-warned-projects)))
+    (when-let* ((globbed (seq-filter
+                          (lambda (entry) (string-match-p "[][*?]" entry))
+                          (projectile-dirconfig-keep cfg))))
+      (puthash project-root t projectile--glob-keep-warned-projects)
+      (display-warning
+       'projectile
+       (format "%s contains `+' entries with glob metacharacters: %s.  \
+The `+' prefix is for subdirectory paths only; globs are not expanded \
+and the entries are silently coerced to directory names.  Use a plain \
+directory or move the pattern to a `-'/`!' rule."
+               (expand-file-name projectile-dirconfig-file project-root)
+               (mapconcat (lambda (s) (format "`%s'" s)) globbed ", "))
+       :warning))))
 
 (defun projectile--dirconfig-classify-line (line)
   "Classify LINE from a dirconfig file.
@@ -2268,14 +2294,10 @@ compatibility but are tracked separately so callers can warn."
 Return a `projectile-dirconfig' or nil if the file doesn't exist."
   (let ((dirconfig (projectile-dirconfig-file)))
     (when (projectile-file-exists-p dirconfig)
-      (let ((cfg (projectile--parse-dirconfig-string
-                  (with-temp-buffer
-                    (insert-file-contents dirconfig)
-                    (buffer-string)))))
-        (dolist (entry (projectile-dirconfig-keep cfg))
-          (when (string-match-p "[*?[]" entry)
-            (projectile--warn-glob-in-keep-entry entry dirconfig)))
-        cfg))))
+      (projectile--parse-dirconfig-string
+       (with-temp-buffer
+         (insert-file-contents dirconfig)
+         (buffer-string))))))
 
 (defun projectile--maybe-warn-prefixless-entries (project-root cfg)
   "Warn once per session about prefix-less ignore entries in CFG for PROJECT-ROOT.
@@ -2326,15 +2348,18 @@ dirconfig file's modification time changes."
          (cached (gethash project-root projectile--dirconfig-cache))
          (attrs (file-attributes dirconfig))
          (mtime (when attrs (file-attribute-modification-time attrs)))
-         (result (if (and cached mtime (equal (car cached) mtime))
-                     (cdr cached)
+         (result (if (and cached mtime
+                          (equal (nth 0 cached) dirconfig)
+                          (equal (nth 1 cached) mtime))
+                     (nth 2 cached)
                    (let ((parsed (projectile--parse-dirconfig-file-uncached)))
                      (when mtime
                        (puthash project-root
-                                (cons mtime parsed)
+                                (list dirconfig mtime parsed)
                                 projectile--dirconfig-cache))
                      parsed))))
     (projectile--maybe-warn-prefixless-entries project-root result)
+    (projectile--maybe-warn-glob-keep-entries project-root result)
     result))
 
 (defun projectile-expand-root (name &optional dir)
diff --git a/test/projectile-test.el b/test/projectile-test.el
index 9516ff1..b62224a 100644
--- a/test/projectile-test.el
+++ b/test/projectile-test.el
@@ -559,6 +559,10 @@ Just delegates OPERATION and ARGS for all operations except for`shell-command`'.
               :to-equal '(:legacy-ignore . "#may-be-a-comment")))))
 
 (describe "projectile-parse-dirconfig-file"
+  (before-each
+    (clrhash projectile--dirconfig-cache)
+    (clrhash projectile--glob-keep-warned-projects)
+    (clrhash projectile--prefixless-dirconfig-warned-projects))
   (it "parses dirconfig and returns directories to ignore and keep"
     (spy-on 'file-exists-p :and-return-value t)
     (spy-on 'file-truename :and-call-fake (lambda (filename) filename))
@@ -614,13 +618,14 @@ Just delegates OPERATION and ARGS for all operations except for`shell-command`'.
     (let ((projectile-dirconfig-comment-prefix ?#))
       (expect (projectile-parse-dirconfig-file)
               :to-equal (make-projectile-dirconfig :ignore '("keep-this")))))
-  (it "warns when a + keep entry contains glob metacharacters"
+  (it "warns once per project even when multiple + entries contain globs"
     (spy-on 'file-exists-p :and-return-value t)
     (spy-on 'insert-file-contents :and-call-fake
             (lambda (_filename)
-              (save-excursion (insert "+/*.json\n+/src\n"))))
+              (save-excursion (insert "+/*.json\n+/src\n+/[abc]/lib\n"))))
     (spy-on 'display-warning)
     (projectile-parse-dirconfig-file)
+    (projectile-parse-dirconfig-file)
     (expect 'display-warning :to-have-been-called-times 1))
   (it "does not warn for plain + subdirectory entries"
     (spy-on 'file-exists-p :and-return-value t)
@@ -741,7 +746,23 @@ Just delegates OPERATION and ARGS for all operations except for`shell-command`'.
         (projectile-parse-dirconfig-file)
         (expect (gethash root projectile--dirconfig-cache) :not :to-be nil)
         (projectile-invalidate-cache nil)
-        (expect (gethash root projectile--dirconfig-cache) :to-be nil))))))
+        (expect (gethash root projectile--dirconfig-cache) :to-be nil)))))
+  (it "re-parses when projectile-dirconfig-file points to a different file"
+    (projectile-test-with-sandbox
+     (projectile-test-with-files
+      ("project/")
+      (let ((root (file-truename (expand-file-name "project/"))))
+        (with-temp-file (expand-file-name ".projectile" root)
+          (insert "-foo\n"))
+        (with-temp-file (expand-file-name ".projectile-alt" root)
+          (insert "-bar\n"))
+        (spy-on 'projectile-project-root :and-return-value root)
+        (let ((projectile-dirconfig-file ".projectile"))
+          (expect (projectile-dirconfig-ignore (projectile-parse-dirconfig-file))
+                  :to-equal '("foo")))
+        (let ((projectile-dirconfig-file ".projectile-alt"))
+          (expect (projectile-dirconfig-ignore (projectile-parse-dirconfig-file))
+                  :to-equal '("bar"))))))))
 
 (describe "prefix-less dirconfig warning"
   (before-each