GNU bug report logs - #66649
29.1; `project-remember-projects-under' behavior doesn't match its doc

Previous Next

Package: emacs;

Reported by: Damien Cassou <damien <at> cassou.me>

Date: Fri, 20 Oct 2023 11:50:01 UTC

Severity: normal

Found in version 29.1

Done: Dmitry Gutov <dgutov <at> yandex.ru>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 66649 in the body.
You can then email your comments to 66649 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#66649; Package emacs. (Fri, 20 Oct 2023 11:50:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Damien Cassou <damien <at> cassou.me>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Fri, 20 Oct 2023 11:50:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Damien Cassou <damien <at> cassou.me>
To: bug-gnu-emacs <at> gnu.org
Cc: Dmitry Gutov <dgutov <at> yandex.ru>
Subject: 29.1; `project-remember-projects-under' behavior doesn't match its doc
Date: Fri, 20 Oct 2023 13:48:04 +0200
Hi,

the documentation of `project-remember-projects-under' is:

      "Index all projects below a directory DIR.  If RECURSIVE is
    non-nil, recurse into all subdirectories to find more projects.
    After finishing, a message is printed summarizing the progress.  The
    function returns the number of detected projects."

Regardless of the value of RECURSIVE, I understand from the above that
all child directories of the DIR argument will be investigated. The doc
doesn't say anything about investigating if DIR is itself a project or
not so I think it would make sense if the function wasn't.

But the code says otherwise (as far as I understand it):

(defun project-remember-projects-under (dir &optional recursive)
  (let ((queue (list dir)))
    ;; …
    (while queue
      (when-let ((subdir (pop queue))
                 ((file-directory-p subdir)))
        ;; maybe register `subdir' as a project
        ;; …
        (when (and recursive (file-directory-p subdir))
          (setq queue (nconc (directory-files subdir …) queue)))))))

The code above seems to investigate DIR first and, if RECURSIVE is
non-nil, look at the directories below it.

Also, the second check (file-directory-p subdir) seems unnecessary
because of the first one.

There is a part of the code I don't understand:

  (unless (eq recursive 'in-progress)

It seems nowhere in the code nor in the documentation do we say anything
about this 'in-progress special value. Is it a left over from a previous
(recursive) version of the algorithm?

Best

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#66649; Package emacs. (Fri, 20 Oct 2023 15:48:01 GMT) Full text and rfc822 format available.

Message #8 received at 66649 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Damien Cassou <damien <at> cassou.me>, 66649 <at> debbugs.gnu.org,
 "Philip K." <philipk <at> posteo.net>
Subject: Re: bug#66649: 29.1; `project-remember-projects-under' behavior
 doesn't match its doc
Date: Fri, 20 Oct 2023 18:46:41 +0300
Hi Damien, thanks for the report.

On 20/10/2023 14:48, Damien Cassou wrote:
> the documentation of `project-remember-projects-under' is:
> 
>        "Index all projects below a directory DIR.  If RECURSIVE is
>      non-nil, recurse into all subdirectories to find more projects.
>      After finishing, a message is printed summarizing the progress.  The
>      function returns the number of detected projects."
> 
> Regardless of the value of RECURSIVE, I understand from the above that
> all child directories of the DIR argument will be investigated. The doc
> doesn't say anything about investigating if DIR is itself a project or
> not so I think it would make sense if the function wasn't.
> 
> But the code says otherwise (as far as I understand it):
> 
> (defun project-remember-projects-under (dir &optional recursive)
>    (let ((queue (list dir)))
>      ;; …
>      (while queue
>        (when-let ((subdir (pop queue))
>                   ((file-directory-p subdir)))
>          ;; maybe register `subdir' as a project
>          ;; …
>          (when (and recursive (file-directory-p subdir))
>            (setq queue (nconc (directory-files subdir …) queue)))))))
> 
> The code above seems to investigate DIR first and, if RECURSIVE is
> non-nil, look at the directories below it.
> 
> Also, the second check (file-directory-p subdir) seems unnecessary
> because of the first one.
> 
> There is a part of the code I don't understand:
> 
>    (unless (eq recursive 'in-progress)
> 
> It seems nowhere in the code nor in the documentation do we say anything
> about this 'in-progress special value. Is it a left over from a previous
> (recursive) version of the algorithm?

Philip, could you look into this?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#66649; Package emacs. (Wed, 01 Nov 2023 13:14:02 GMT) Full text and rfc822 format available.

Message #11 received at 66649 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: Damien Cassou <damien <at> cassou.me>, 66649 <at> debbugs.gnu.org
Subject: Re: bug#66649: 29.1; `project-remember-projects-under' behavior
 doesn't match its doc
Date: Wed, 01 Nov 2023 13:12:27 +0000
[Message part 1 (text/plain, inline)]
Dmitry Gutov <dgutov <at> yandex.ru> writes:

> Hi Damien, thanks for the report.
>
> On 20/10/2023 14:48, Damien Cassou wrote:
>> the documentation of `project-remember-projects-under' is:
>>        "Index all projects below a directory DIR.  If RECURSIVE is
>>      non-nil, recurse into all subdirectories to find more projects.
>>      After finishing, a message is printed summarizing the progress.  The
>>      function returns the number of detected projects."
>> Regardless of the value of RECURSIVE, I understand from the above
>> that
>> all child directories of the DIR argument will be investigated. The doc
>> doesn't say anything about investigating if DIR is itself a project or
>> not so I think it would make sense if the function wasn't.
>> But the code says otherwise (as far as I understand it):
>> (defun project-remember-projects-under (dir &optional recursive)
>>    (let ((queue (list dir)))
>>      ;; …
>>      (while queue
>>        (when-let ((subdir (pop queue))
>>                   ((file-directory-p subdir)))
>>          ;; maybe register `subdir' as a project
>>          ;; …
>>          (when (and recursive (file-directory-p subdir))
>>            (setq queue (nconc (directory-files subdir …) queue)))))))
>> The code above seems to investigate DIR first and, if RECURSIVE is
>> non-nil, look at the directories below it.
>> Also, the second check (file-directory-p subdir) seems unnecessary
>> because of the first one.
>> There is a part of the code I don't understand:
>>    (unless (eq recursive 'in-progress)
>> It seems nowhere in the code nor in the documentation do we say
>> anything
>> about this 'in-progress special value. Is it a left over from a previous
>> (recursive) version of the algorithm?
>
> Philip, could you look into this?

One idea would be to just simplify the entire implementation by relying
on directory-files and directory-files-recursively.  Say something like
this:

[Message part 2 (text/plain, inline)]
diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index fda1081eb62..e0d5e706e82 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1821,35 +1821,28 @@ project-remember-projects-under
 projects."
   (interactive "DDirectory: \nP")
   (project--ensure-read-project-list)
-  (let ((queue (list dir))
-        (count 0)
-        (known (make-hash-table
-                :size (* 2 (length project--list))
-                :test #'equal )))
+  (let ((dirs (if recursive
+                  (directory-files-recursively dir "" t #'file-directory-p)
+                (cl-delete-if-not #'file-directory-p (directory-files dir t))))
+        (known (make-hash-table :size (* 2 (length project--list))
+                                :test #'equal))
+        (count 0))
     (dolist (project (mapcar #'car project--list))
       (puthash project t known))
-    (while queue
-      (when-let ((subdir (pop queue))
-                 ((file-directory-p subdir)))
-        (when-let ((project (project--find-in-directory subdir))
-                   (project-root (project-root project))
-                   ((not (gethash project-root known))))
-          (project-remember-project project t)
-          (puthash project-root t known)
-          (message "Found %s..." project-root)
-          (setq count (1+ count)))
-        (when (and recursive (file-directory-p subdir))
-          (setq queue
-                (nconc
-                 (directory-files
-                  subdir t directory-files-no-dot-files-regexp t)
-                 queue)))))
-    (unless (eq recursive 'in-progress)
-      (if (zerop count)
-          (message "No projects were found")
-        (project--write-project-list)
-        (message "%d project%s were found"
-                 count (if (= count 1) "" "s"))))
+    (dolist (subdir dirs)
+      (when-let (((file-directory-p subdir))
+                 (project (project--find-in-directory subdir))
+                 (project-root (project-root project))
+                 ((not (gethash project-root known))))
+        (project-remember-project project t)
+        (puthash project-root t known)
+        (message "Found %s..." project-root)
+        (setq count (1+ count))))
+    (if (zerop count)
+        (message "No projects were found")
+      (project--write-project-list)
+      (message "%d project%s were found"
+               count (if (= count 1) "" "s")))
     count))
 
 (defun project-forget-zombie-projects ()

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#66649; Package emacs. (Wed, 01 Nov 2023 19:05:01 GMT) Full text and rfc822 format available.

Message #14 received at 66649 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Damien Cassou <damien <at> cassou.me>, 66649 <at> debbugs.gnu.org
Subject: Re: bug#66649: 29.1; `project-remember-projects-under' behavior
 doesn't match its doc
Date: Wed, 1 Nov 2023 21:04:00 +0200
On 01/11/2023 15:12, Philip Kaludercic wrote:
> One idea would be to just simplify the entire implementation by relying
> on directory-files and directory-files-recursively.  Say something like
> this:

Looks, good, just once thing:

> +                  (directory-files-recursively dir "" t #'file-directory-p)

The argument PREDICATE is not in Emacs 26.1 (which should remain 
compatible). But it doesn't help anyway, because that predicate is only 
used to determine whether to recurse into a subdirectory, and not to 
filter out files. So the full list of all files is generated anyway.

If the performance still looks okay to you, I have no objections. Just 
remove that last argument, and it's good to install.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#66649; Package emacs. (Wed, 01 Nov 2023 21:38:01 GMT) Full text and rfc822 format available.

Message #17 received at 66649 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: Damien Cassou <damien <at> cassou.me>, 66649 <at> debbugs.gnu.org
Subject: Re: bug#66649: 29.1; `project-remember-projects-under' behavior
 doesn't match its doc
Date: Wed, 01 Nov 2023 21:36:25 +0000
[Message part 1 (text/plain, inline)]
Dmitry Gutov <dgutov <at> yandex.ru> writes:

> On 01/11/2023 15:12, Philip Kaludercic wrote:
>> One idea would be to just simplify the entire implementation by relying
>> on directory-files and directory-files-recursively.  Say something like
>> this:
>
> Looks, good, just once thing:
>
>> +                  (directory-files-recursively dir "" t #'file-directory-p)
>
> The argument PREDICATE is not in Emacs 26.1 (which should remain
> compatible). But it doesn't help anyway, because that predicate is
> only used to determine whether to recurse into a subdirectory, and not
> to filter out files. So the full list of all files is generated
> anyway.

OK, here is the updated patch:

[0001-Simplify-project-remember-projects-under.patch (text/x-diff, inline)]
From cf1541ddba61e3c6106a133a9b6b662f0f34749d Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk <at> posteo.net>
Date: Wed, 1 Nov 2023 22:34:28 +0100
Subject: [PATCH] Simplify 'project-remember-projects-under'

* lisp/progmodes/project.el (project-remember-projects-under): Instead
of traversing the directories manually, re-use
`directory-files-recursively' to reduce complexity.  (Bug#66649)
---
 lisp/progmodes/project.el | 47 +++++++++++++++++----------------------
 1 file changed, 20 insertions(+), 27 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index fda1081eb62..935f7a7b873 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1821,35 +1821,28 @@ project-remember-projects-under
 projects."
   (interactive "DDirectory: \nP")
   (project--ensure-read-project-list)
-  (let ((queue (list dir))
-        (count 0)
-        (known (make-hash-table
-                :size (* 2 (length project--list))
-                :test #'equal )))
+  (let ((dirs (if recursive
+                  (directory-files-recursively dir "" t)
+                (directory-files dir t)))
+        (known (make-hash-table :size (* 2 (length project--list))
+                                :test #'equal))
+        (count 0))
     (dolist (project (mapcar #'car project--list))
       (puthash project t known))
-    (while queue
-      (when-let ((subdir (pop queue))
-                 ((file-directory-p subdir)))
-        (when-let ((project (project--find-in-directory subdir))
-                   (project-root (project-root project))
-                   ((not (gethash project-root known))))
-          (project-remember-project project t)
-          (puthash project-root t known)
-          (message "Found %s..." project-root)
-          (setq count (1+ count)))
-        (when (and recursive (file-directory-p subdir))
-          (setq queue
-                (nconc
-                 (directory-files
-                  subdir t directory-files-no-dot-files-regexp t)
-                 queue)))))
-    (unless (eq recursive 'in-progress)
-      (if (zerop count)
-          (message "No projects were found")
-        (project--write-project-list)
-        (message "%d project%s were found"
-                 count (if (= count 1) "" "s"))))
+    (dolist (subdir dirs)
+      (when-let (((file-directory-p subdir))
+                 (project (project--find-in-directory subdir))
+                 (project-root (project-root project))
+                 ((not (gethash project-root known))))
+        (project-remember-project project t)
+        (puthash project-root t known)
+        (message "Found %s..." project-root)
+        (setq count (1+ count))))
+    (if (zerop count)
+        (message "No projects were found")
+      (project--write-project-list)
+      (message "%d project%s were found"
+               count (if (= count 1) "" "s")))
     count))
 
 (defun project-forget-zombie-projects ()
-- 
2.39.2

[Message part 3 (text/plain, inline)]

> If the performance still looks okay to you, I have no objections. Just
> remove that last argument, and it's good to install.

I'd say that Damien can try it out and report if it works well enough.
I worry that the files returned by `directory-files-recursively' might
slow down the function considerably.  Perhaps it might be better to
write a little function just for project.el to only generate a list of
directories.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#66649; Package emacs. (Wed, 01 Nov 2023 22:41:02 GMT) Full text and rfc822 format available.

Message #20 received at 66649 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Damien Cassou <damien <at> cassou.me>, 66649 <at> debbugs.gnu.org
Subject: Re: bug#66649: 29.1; `project-remember-projects-under' behavior
 doesn't match its doc
Date: Thu, 2 Nov 2023 00:39:52 +0200
On 01/11/2023 23:36, Philip Kaludercic wrote:
>> If the performance still looks okay to you, I have no objections. Just
>> remove that last argument, and it's good to install.
> I'd say that Damien can try it out and report if it works well enough.

Let's wait, sure.

> I worry that the files returned by `directory-files-recursively' might
> slow down the function considerably.  Perhaps it might be better to
> write a little function just for project.el to only generate a list of
> directories.

It might, or it might not: the implementation of 'project-find-dir' is 
pretty unoptimized in this regard, and yet I'm still waiting for anyone 
to complain.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#66649; Package emacs. (Thu, 02 Nov 2023 19:59:01 GMT) Full text and rfc822 format available.

Message #23 received at 66649 <at> debbugs.gnu.org (full text, mbox):

From: Damien Cassou <damien <at> cassou.me>
To: Dmitry Gutov <dgutov <at> yandex.ru>, Philip Kaludercic <philipk <at> posteo.net>
Cc: 66649 <at> debbugs.gnu.org
Subject: Re: bug#66649: 29.1; `project-remember-projects-under' behavior
 doesn't match its doc
Date: Thu, 02 Nov 2023 20:58:05 +0100
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 01/11/2023 23:36, Philip Kaludercic wrote:
>> I'd say that Damien can try it out and report if it works well enough.
>
> Let's wait, sure.

this is working great, thank you. The code is also simpler to understand
in my opinion. Good job.

If I may, the code of `project-remember-projects-under' seems to suffer
from a similar mismatch between the docstring and implementation when
RECURSIVE is nil: only the DIR directory is tested and not "known
projects below a directory DIR".

Best,

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#66649; Package emacs. (Thu, 02 Nov 2023 20:43:01 GMT) Full text and rfc822 format available.

Message #26 received at 66649 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Damien Cassou <damien <at> cassou.me>, Philip Kaludercic <philipk <at> posteo.net>
Cc: 66649 <at> debbugs.gnu.org
Subject: Re: bug#66649: 29.1; `project-remember-projects-under' behavior
 doesn't match its doc
Date: Thu, 2 Nov 2023 22:41:41 +0200
On 02/11/2023 21:58, Damien Cassou wrote:
> If I may, the code of `project-remember-projects-under' seems to suffer
> from a similar mismatch between the docstring and implementation when
> RECURSIVE is nil: only the DIR directory is tested and not "known
> projects below a directory DIR".

Have you seen this problem when testing it?

The last line in

+  (let ((dirs (if recursive
+                  (directory-files-recursively dir "" t)
+                (directory-files dir t)))

seems to do the job of including the direct subdirectories in the search.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#66649; Package emacs. (Fri, 03 Nov 2023 13:02:02 GMT) Full text and rfc822 format available.

Message #29 received at 66649 <at> debbugs.gnu.org (full text, mbox):

From: Damien Cassou <damien <at> cassou.me>
To: Dmitry Gutov <dgutov <at> yandex.ru>, Philip Kaludercic <philipk <at> posteo.net>
Cc: 66649 <at> debbugs.gnu.org
Subject: Re: bug#66649: 29.1; `project-remember-projects-under' behavior
 doesn't match its doc
Date: Fri, 03 Nov 2023 14:00:54 +0100
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 02/11/2023 21:58, Damien Cassou wrote:
>> If I may, the code of `project-remember-projects-under' seems to suffer
>> from a similar mismatch between the docstring and implementation when
>> RECURSIVE is nil: only the DIR directory is tested and not "known
>> projects below a directory DIR".
>
> Have you seen this problem when testing it?

The patch is working perfectly find, thank you. I was talking about an
unrelated (but similar) problem in a different function:
`project-forget-projects-under'. My message incorrectly referred to the
function you already fixed. Sorry for the confusion.

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#66649; Package emacs. (Wed, 08 Nov 2023 08:15:01 GMT) Full text and rfc822 format available.

Message #32 received at 66649 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Damien Cassou <damien <at> cassou.me>
Cc: 66649 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#66649: 29.1; `project-remember-projects-under' behavior
 doesn't match its doc
Date: Wed, 08 Nov 2023 08:13:16 +0000
[Message part 1 (text/plain, inline)]
Damien Cassou <damien <at> cassou.me> writes:

> Dmitry Gutov <dgutov <at> yandex.ru> writes:
>> On 02/11/2023 21:58, Damien Cassou wrote:
>>> If I may, the code of `project-remember-projects-under' seems to suffer
>>> from a similar mismatch between the docstring and implementation when
>>> RECURSIVE is nil: only the DIR directory is tested and not "known
>>> projects below a directory DIR".
>>
>> Have you seen this problem when testing it?
>
> The patch is working perfectly find, thank you. I was talking about an
> unrelated (but similar) problem in a different function:
> `project-forget-projects-under'. My message incorrectly referred to the
> function you already fixed. Sorry for the confusion.

I don't think this is the same problem, in
`project-forget-projects-under' there is no manual recursive descent,
just some duplicated code.  We could also re-write it to look like this:

[Message part 2 (text/plain, inline)]
diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 95db9d0ef4c..5f1cce160b2 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1905,15 +1905,12 @@ project-forget-projects-under
 forgotten projects."
   (interactive "DDirectory: \nP")
   (let ((count 0))
-    (if recursive
-        (dolist (proj (project-known-project-roots))
-          (when (file-in-directory-p proj dir)
-            (project-forget-project proj)
-            (setq count (1+ count))))
-      (dolist (proj (project-known-project-roots))
-        (when (file-equal-p (file-name-directory proj) dir)
-          (project-forget-project proj)
-          (setq count (1+ count)))))
+    (dolist (proj (project-known-project-roots))
+      (when (if recursive
+                (file-in-directory-p proj dir)
+              (file-equal-p (file-name-directory proj) dir))
+        (project-forget-project proj)
+        (setq count (1+ count))))
     (if (zerop count)
         (message "No projects were forgotten")
       (project--write-project-list)
[Message part 3 (text/plain, inline)]
But that would incur a branch in every iteration of `dolist'.

Either way, I'll push the first patch to master since there haven't been
any objections to that change.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#66649; Package emacs. (Wed, 08 Nov 2023 19:58:01 GMT) Full text and rfc822 format available.

Message #35 received at 66649 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Philip Kaludercic <philipk <at> posteo.net>, Damien Cassou <damien <at> cassou.me>
Cc: 66649 <at> debbugs.gnu.org
Subject: Re: bug#66649: 29.1; `project-remember-projects-under' behavior
 doesn't match its doc
Date: Wed, 8 Nov 2023 21:56:57 +0200
On 08/11/2023 10:13, Philip Kaludercic wrote:
> I don't think this is the same problem, in
> `project-forget-projects-under' there is no manual recursive descent,
> just some duplicated code.  We could also re-write it to look like this:
> 
> 
> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
> index 95db9d0ef4c..5f1cce160b2 100644
> --- a/lisp/progmodes/project.el
> +++ b/lisp/progmodes/project.el
> @@ -1905,15 +1905,12 @@ project-forget-projects-under
>   forgotten projects."
>     (interactive "DDirectory: \nP")
>     (let ((count 0))
> -    (if recursive
> -        (dolist (proj (project-known-project-roots))
> -          (when (file-in-directory-p proj dir)
> -            (project-forget-project proj)
> -            (setq count (1+ count))))
> -      (dolist (proj (project-known-project-roots))
> -        (when (file-equal-p (file-name-directory proj) dir)
> -          (project-forget-project proj)
> -          (setq count (1+ count)))))
> +    (dolist (proj (project-known-project-roots))
> +      (when (if recursive
> +                (file-in-directory-p proj dir)
> +              (file-equal-p (file-name-directory proj) dir))
> +        (project-forget-project proj)
> +        (setq count (1+ count))))
>       (if (zerop count)
>           (message "No projects were forgotten")
>         (project--write-project-list)
> 
> 
> But that would incur a branch in every iteration of `dolist'.

LGTM too. The branch-per-iteration is unlikely to move a needle in any 
realistic scenario.

Up to you, whether to install this or keep the original version.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#66649; Package emacs. (Wed, 08 Nov 2023 20:00:02 GMT) Full text and rfc822 format available.

Message #38 received at 66649 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: Damien Cassou <damien <at> cassou.me>, 66649 <at> debbugs.gnu.org
Subject: Re: bug#66649: 29.1; `project-remember-projects-under' behavior
 doesn't match its doc
Date: Wed, 08 Nov 2023 19:58:43 +0000
Dmitry Gutov <dgutov <at> yandex.ru> writes:

> On 08/11/2023 10:13, Philip Kaludercic wrote:
>> I don't think this is the same problem, in
>> `project-forget-projects-under' there is no manual recursive descent,
>> just some duplicated code.  We could also re-write it to look like this:
>> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
>> index 95db9d0ef4c..5f1cce160b2 100644
>> --- a/lisp/progmodes/project.el
>> +++ b/lisp/progmodes/project.el
>> @@ -1905,15 +1905,12 @@ project-forget-projects-under
>>   forgotten projects."
>>     (interactive "DDirectory: \nP")
>>     (let ((count 0))
>> -    (if recursive
>> -        (dolist (proj (project-known-project-roots))
>> -          (when (file-in-directory-p proj dir)
>> -            (project-forget-project proj)
>> -            (setq count (1+ count))))
>> -      (dolist (proj (project-known-project-roots))
>> -        (when (file-equal-p (file-name-directory proj) dir)
>> -          (project-forget-project proj)
>> -          (setq count (1+ count)))))
>> +    (dolist (proj (project-known-project-roots))
>> +      (when (if recursive
>> +                (file-in-directory-p proj dir)
>> +              (file-equal-p (file-name-directory proj) dir))
>> +        (project-forget-project proj)
>> +        (setq count (1+ count))))
>>       (if (zerop count)
>>           (message "No projects were forgotten")
>>         (project--write-project-list)
>> But that would incur a branch in every iteration of `dolist'.
>
> LGTM too. The branch-per-iteration is unlikely to move a needle in any
> realistic scenario.
>
> Up to you, whether to install this or keep the original version.

I don't see a need, this is basically an aesthetic change.  Should we
close the bug report?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#66649; Package emacs. (Wed, 08 Nov 2023 20:17:02 GMT) Full text and rfc822 format available.

Message #41 received at 66649 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Damien Cassou <damien <at> cassou.me>
Cc: 66649 <at> debbugs.gnu.org, "Philip K." <philipk <at> posteo.net>
Subject: Re: bug#66649: 29.1; `project-remember-projects-under' behavior
 doesn't match its doc
Date: Wed, 8 Nov 2023 22:16:04 +0200
On 08/11/2023 21:58, Philip Kaludercic wrote:
> Dmitry Gutov<dgutov <at> yandex.ru>  writes:
> 
>> On 08/11/2023 10:13, Philip Kaludercic wrote:
>>> I don't think this is the same problem, in
>>> `project-forget-projects-under' there is no manual recursive descent,
>>> just some duplicated code.  We could also re-write it to look like this:
>>> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
>>> index 95db9d0ef4c..5f1cce160b2 100644
>>> --- a/lisp/progmodes/project.el
>>> +++ b/lisp/progmodes/project.el
>>> @@ -1905,15 +1905,12 @@ project-forget-projects-under
>>>    forgotten projects."
>>>      (interactive "DDirectory: \nP")
>>>      (let ((count 0))
>>> -    (if recursive
>>> -        (dolist (proj (project-known-project-roots))
>>> -          (when (file-in-directory-p proj dir)
>>> -            (project-forget-project proj)
>>> -            (setq count (1+ count))))
>>> -      (dolist (proj (project-known-project-roots))
>>> -        (when (file-equal-p (file-name-directory proj) dir)
>>> -          (project-forget-project proj)
>>> -          (setq count (1+ count)))))
>>> +    (dolist (proj (project-known-project-roots))
>>> +      (when (if recursive
>>> +                (file-in-directory-p proj dir)
>>> +              (file-equal-p (file-name-directory proj) dir))
>>> +        (project-forget-project proj)
>>> +        (setq count (1+ count))))
>>>        (if (zerop count)
>>>            (message "No projects were forgotten")
>>>          (project--write-project-list)
>>> But that would incur a branch in every iteration of `dolist'.
>> LGTM too. The branch-per-iteration is unlikely to move a needle in any
>> realistic scenario.
>>
>> Up to you, whether to install this or keep the original version.
> I don't see a need, this is basically an aesthetic change.  Should we
> close the bug report?

Damien, is there anything else here to do?

Did you perhaps also (or instead) saw a problem with either of the 
docstrings? I'm not sure if I understood the last complaint correctly.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#66649; Package emacs. (Wed, 08 Nov 2023 21:15:01 GMT) Full text and rfc822 format available.

Message #44 received at 66649 <at> debbugs.gnu.org (full text, mbox):

From: Damien Cassou <damien <at> cassou.me>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 66649 <at> debbugs.gnu.org, "Philip K." <philipk <at> posteo.net>
Subject: Re: bug#66649: 29.1; `project-remember-projects-under' behavior
 doesn't match its doc
Date: Wed, 08 Nov 2023 22:13:34 +0100
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> Damien, is there anything else here to do?
>
> Did you perhaps also (or instead) saw a problem with either of the 
> docstrings? I'm not sure if I understood the last complaint correctly.


The bug can be closed, no problem.

Thanks everyone for your great work.


-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill




Reply sent to Dmitry Gutov <dgutov <at> yandex.ru>:
You have taken responsibility. (Wed, 08 Nov 2023 21:18:02 GMT) Full text and rfc822 format available.

Notification sent to Damien Cassou <damien <at> cassou.me>:
bug acknowledged by developer. (Wed, 08 Nov 2023 21:18:02 GMT) Full text and rfc822 format available.

Message #49 received at 66649-done <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Damien Cassou <damien <at> cassou.me>
Cc: 66649-done <at> debbugs.gnu.org, "Philip K." <philipk <at> posteo.net>
Subject: Re: bug#66649: 29.1; `project-remember-projects-under' behavior
 doesn't match its doc
Date: Wed, 8 Nov 2023 23:16:15 +0200
On 08/11/2023 23:13, Damien Cassou wrote:
> Dmitry Gutov<dgutov <at> yandex.ru>  writes:
>> Damien, is there anything else here to do?
>>
>> Did you perhaps also (or instead) saw a problem with either of the
>> docstrings? I'm not sure if I understood the last complaint correctly.
> 
> The bug can be closed, no problem.
> 
> Thanks everyone for your great work.

Thanks for reporting!




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Thu, 07 Dec 2023 12:24:08 GMT) Full text and rfc822 format available.

This bug report was last modified 1 year and 251 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.