* bug#66649: 29.1; `project-remember-projects-under' behavior doesn't match its doc
@ 2023-10-20 11:48 Damien Cassou
2023-10-20 15:46 ` Dmitry Gutov
0 siblings, 1 reply; 15+ messages in thread
From: Damien Cassou @ 2023-10-20 11:48 UTC (permalink / raw)
To: 66649; +Cc: Dmitry Gutov
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#66649: 29.1; `project-remember-projects-under' behavior doesn't match its doc
2023-10-20 11:48 bug#66649: 29.1; `project-remember-projects-under' behavior doesn't match its doc Damien Cassou
@ 2023-10-20 15:46 ` Dmitry Gutov
2023-11-01 13:12 ` Philip Kaludercic
0 siblings, 1 reply; 15+ messages in thread
From: Dmitry Gutov @ 2023-10-20 15:46 UTC (permalink / raw)
To: Damien Cassou, 66649, Philip K.
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?
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#66649: 29.1; `project-remember-projects-under' behavior doesn't match its doc
2023-10-20 15:46 ` Dmitry Gutov
@ 2023-11-01 13:12 ` Philip Kaludercic
2023-11-01 19:04 ` Dmitry Gutov
0 siblings, 1 reply; 15+ messages in thread
From: Philip Kaludercic @ 2023-11-01 13:12 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: Damien Cassou, 66649
[-- Attachment #1: Type: text/plain, Size: 1982 bytes --]
Dmitry Gutov <dgutov@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:
[-- Attachment #2: Type: text/plain, Size: 2496 bytes --]
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 ()
^ permalink raw reply related [flat|nested] 15+ messages in thread
* bug#66649: 29.1; `project-remember-projects-under' behavior doesn't match its doc
2023-11-01 13:12 ` Philip Kaludercic
@ 2023-11-01 19:04 ` Dmitry Gutov
2023-11-01 21:36 ` Philip Kaludercic
0 siblings, 1 reply; 15+ messages in thread
From: Dmitry Gutov @ 2023-11-01 19:04 UTC (permalink / raw)
To: Philip Kaludercic; +Cc: Damien Cassou, 66649
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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#66649: 29.1; `project-remember-projects-under' behavior doesn't match its doc
2023-11-01 19:04 ` Dmitry Gutov
@ 2023-11-01 21:36 ` Philip Kaludercic
2023-11-01 22:39 ` Dmitry Gutov
0 siblings, 1 reply; 15+ messages in thread
From: Philip Kaludercic @ 2023-11-01 21:36 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: Damien Cassou, 66649
[-- Attachment #1: Type: text/plain, Size: 684 bytes --]
Dmitry Gutov <dgutov@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:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Simplify-project-remember-projects-under.patch --]
[-- Type: text/x-diff, Size: 2976 bytes --]
From cf1541ddba61e3c6106a133a9b6b662f0f34749d Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@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
[-- Attachment #3: Type: text/plain, Size: 425 bytes --]
> 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.
^ permalink raw reply related [flat|nested] 15+ messages in thread
* bug#66649: 29.1; `project-remember-projects-under' behavior doesn't match its doc
2023-11-01 21:36 ` Philip Kaludercic
@ 2023-11-01 22:39 ` Dmitry Gutov
2023-11-02 19:58 ` Damien Cassou
0 siblings, 1 reply; 15+ messages in thread
From: Dmitry Gutov @ 2023-11-01 22:39 UTC (permalink / raw)
To: Philip Kaludercic; +Cc: Damien Cassou, 66649
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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#66649: 29.1; `project-remember-projects-under' behavior doesn't match its doc
2023-11-01 22:39 ` Dmitry Gutov
@ 2023-11-02 19:58 ` Damien Cassou
2023-11-02 20:41 ` Dmitry Gutov
0 siblings, 1 reply; 15+ messages in thread
From: Damien Cassou @ 2023-11-02 19:58 UTC (permalink / raw)
To: Dmitry Gutov, Philip Kaludercic; +Cc: 66649
Dmitry Gutov <dgutov@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
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#66649: 29.1; `project-remember-projects-under' behavior doesn't match its doc
2023-11-02 19:58 ` Damien Cassou
@ 2023-11-02 20:41 ` Dmitry Gutov
2023-11-03 13:00 ` Damien Cassou
0 siblings, 1 reply; 15+ messages in thread
From: Dmitry Gutov @ 2023-11-02 20:41 UTC (permalink / raw)
To: Damien Cassou, Philip Kaludercic; +Cc: 66649
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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#66649: 29.1; `project-remember-projects-under' behavior doesn't match its doc
2023-11-02 20:41 ` Dmitry Gutov
@ 2023-11-03 13:00 ` Damien Cassou
2023-11-08 8:13 ` Philip Kaludercic
0 siblings, 1 reply; 15+ messages in thread
From: Damien Cassou @ 2023-11-03 13:00 UTC (permalink / raw)
To: Dmitry Gutov, Philip Kaludercic; +Cc: 66649
Dmitry Gutov <dgutov@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
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#66649: 29.1; `project-remember-projects-under' behavior doesn't match its doc
2023-11-03 13:00 ` Damien Cassou
@ 2023-11-08 8:13 ` Philip Kaludercic
2023-11-08 19:56 ` Dmitry Gutov
0 siblings, 1 reply; 15+ messages in thread
From: Philip Kaludercic @ 2023-11-08 8:13 UTC (permalink / raw)
To: Damien Cassou; +Cc: 66649, Dmitry Gutov
[-- Attachment #1: Type: text/plain, Size: 887 bytes --]
Damien Cassou <damien@cassou.me> writes:
> Dmitry Gutov <dgutov@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:
[-- Attachment #2: Type: text/plain, Size: 1064 bytes --]
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)
[-- Attachment #3: Type: text/plain, Size: 168 bytes --]
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.
^ permalink raw reply related [flat|nested] 15+ messages in thread
* bug#66649: 29.1; `project-remember-projects-under' behavior doesn't match its doc
2023-11-08 8:13 ` Philip Kaludercic
@ 2023-11-08 19:56 ` Dmitry Gutov
2023-11-08 19:58 ` Philip Kaludercic
0 siblings, 1 reply; 15+ messages in thread
From: Dmitry Gutov @ 2023-11-08 19:56 UTC (permalink / raw)
To: Philip Kaludercic, Damien Cassou; +Cc: 66649
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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#66649: 29.1; `project-remember-projects-under' behavior doesn't match its doc
2023-11-08 19:56 ` Dmitry Gutov
@ 2023-11-08 19:58 ` Philip Kaludercic
2023-11-08 20:16 ` Dmitry Gutov
0 siblings, 1 reply; 15+ messages in thread
From: Philip Kaludercic @ 2023-11-08 19:58 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: Damien Cassou, 66649
Dmitry Gutov <dgutov@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?
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#66649: 29.1; `project-remember-projects-under' behavior doesn't match its doc
2023-11-08 19:58 ` Philip Kaludercic
@ 2023-11-08 20:16 ` Dmitry Gutov
2023-11-08 21:13 ` Damien Cassou
0 siblings, 1 reply; 15+ messages in thread
From: Dmitry Gutov @ 2023-11-08 20:16 UTC (permalink / raw)
To: Damien Cassou; +Cc: 66649, Philip K.
On 08/11/2023 21:58, Philip Kaludercic wrote:
> Dmitry Gutov<dgutov@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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#66649: 29.1; `project-remember-projects-under' behavior doesn't match its doc
2023-11-08 20:16 ` Dmitry Gutov
@ 2023-11-08 21:13 ` Damien Cassou
2023-11-08 21:16 ` Dmitry Gutov
0 siblings, 1 reply; 15+ messages in thread
From: Damien Cassou @ 2023-11-08 21:13 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 66649, Philip K.
Dmitry Gutov <dgutov@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
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#66649: 29.1; `project-remember-projects-under' behavior doesn't match its doc
2023-11-08 21:13 ` Damien Cassou
@ 2023-11-08 21:16 ` Dmitry Gutov
0 siblings, 0 replies; 15+ messages in thread
From: Dmitry Gutov @ 2023-11-08 21:16 UTC (permalink / raw)
To: Damien Cassou; +Cc: 66649-done, Philip K.
On 08/11/2023 23:13, Damien Cassou wrote:
> Dmitry Gutov<dgutov@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!
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-11-08 21:16 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-20 11:48 bug#66649: 29.1; `project-remember-projects-under' behavior doesn't match its doc Damien Cassou
2023-10-20 15:46 ` Dmitry Gutov
2023-11-01 13:12 ` Philip Kaludercic
2023-11-01 19:04 ` Dmitry Gutov
2023-11-01 21:36 ` Philip Kaludercic
2023-11-01 22:39 ` Dmitry Gutov
2023-11-02 19:58 ` Damien Cassou
2023-11-02 20:41 ` Dmitry Gutov
2023-11-03 13:00 ` Damien Cassou
2023-11-08 8:13 ` Philip Kaludercic
2023-11-08 19:56 ` Dmitry Gutov
2023-11-08 19:58 ` Philip Kaludercic
2023-11-08 20:16 ` Dmitry Gutov
2023-11-08 21:13 ` Damien Cassou
2023-11-08 21:16 ` Dmitry Gutov
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.