project-find-file: switch to include non-tracked files

project-find-file: switch to include non-tracked files

[Manuel Uberti]

Hello everyone,

there is a nice feature in helm-ls-git[1] which lets you browse files
non-tracked by the current VCS (bound to C-c i in helm-ls-git-map). Hitting C-c
i again brings you back to the tracked-files-only browsing.

Is something achievable with project-find-file?

I don't think this is a bug report, but if you prefer for this to be reported as
a bug I will do it.


All the best

[1] https://github.com/emacs-helm/helm-ls-git

-- 
Manuel Uberti
www.manueluberti.eu

Re: project-find-file: switch to include non-tracked files

[Stefan Kangas]

Manuel Uberti <manuel.uberti@inventati.org> writes:

> there is a nice feature in helm-ls-git[1] which lets you browse files
> non-tracked by the current VCS (bound to C-c i in helm-ls-git-map). Hitting C-c
> i again brings you back to the tracked-files-only browsing.

Good idea.  I would definitely use something like that.

> Is something achievable with project-find-file?
>
> I don't think this is a bug report, but if you prefer for this to be reported as
> a bug I will do it.

You can report bugs for feature requests, from Info node '(emacs) Bugs':

    If you think you have found a bug in Emacs, please report it.  We cannot
    promise to fix it, or always to agree that it is a bug, but we certainly
    want to hear about it.  The same applies for new features you would like
    to see added.

If you put "Severity: wishlist" as the first line of your email, it
will be automatically categorized as a feature request, but it is not a
requirement to do that.  It can be categorized manually later.

Re: project-find-file: switch to include non-tracked files

[Dmitry Gutov]

Hi!

On 04.05.2021 16:39, Manuel Uberti wrote:
> there is a nice feature in helm-ls-git[1] which lets you browse files
> non-tracked by the current VCS (bound to C-c i in helm-ls-git-map). Hitting C-c
> i again brings you back to the tracked-files-only browsing.

The current behavior is to include the untracked files (unless they are 
in .gitignore). And you can specify additional ignores through the 
project-vc-ignores variable.

If that is not enough, please outline your usage scenario(s).

Re: project-find-file: switch to include non-tracked files

[Manuel Uberti]

On 04/05/21 16:16, Dmitry Gutov wrote:
> The current behavior is to include the untracked files (unless they are in
> .gitignore). And you can specify additional ignores through the
> project-vc-ignores variable.
> 
> If that is not enough, please outline your usage scenario(s).

Sure, I'll try to explain myself better. I have Git-versioned projects where I
usually track an .envrc.sample file for everyone to use as a starting point on
their own machines. Developers are instructed to copy that file to a local
(i.e., untracked) .envrc file. Since there could (and usually will) be
information we don't want to be tracked on Git (e.g., one's own system-related
customizations), .envrc is listed in .gitignore.

Every now and then, I need to open that .envrc file and add something, but
because it is listed in .gitignore, the file doesn't show up in project-find-file.

What helm-ls-git offers is the possibility to hit C-c i and showing the
untracked files. This doesn't involve changing any setting, it's just a matter
of pressing C-c i whenever is needed. This is useful because generally I want
everything listed in .gitignore to be ignored by project-find-file, but there
are small exceptions such as this where I want to edit an ignored file without
using find-file to get to it.

-- 
Manuel Uberti
www.manueluberti.eu

Re: project-find-file: switch to include non-tracked files

[Dmitry Gutov]

On 04.05.2021 17:55, Manuel Uberti wrote:
> On 04/05/21 16:16, Dmitry Gutov wrote:
>> The current behavior is to include the untracked files (unless they are in
>> .gitignore). And you can specify additional ignores through the
>> project-vc-ignores variable.
>>
>> If that is not enough, please outline your usage scenario(s).
> 
> Sure, I'll try to explain myself better. I have Git-versioned projects where I
> usually track an .envrc.sample file for everyone to use as a starting point on
> their own machines. Developers are instructed to copy that file to a local
> (i.e., untracked) .envrc file. Since there could (and usually will) be
> information we don't want to be tracked on Git (e.g., one's own system-related
> customizations), .envrc is listed in .gitignore.
> 
> Every now and then, I need to open that .envrc file and add something, but
> because it is listed in .gitignore, the file doesn't show up in project-find-file.

Aha! So you want to open an _ignored_ file (which is also untracked, of 
course).

There are two ways to solve this:

- Add "whitelisting" syntax to project-vc-ignores and the 
project-ignores method. Then you edit the value of this variable, 
whitelisting .envrc in the given project (or globally), and 
project-find-file will always include it.

- We add an arg like NO-PROJECT-IGNORES to the project-files method. 
Then project-find-file could use it in (some?) circumstances.

> What helm-ls-git offers is the possibility to hit C-c i and showing the
> untracked files.

Patches or even plain suggestions welcome for the GUI side of this, at 
least. I'm not sure how to add a behavior like this on 'C-c i' to the 
default completing-read interface, which is what project-find-file 
ultimately calls (and can dispatch to alternative UIs like Ivy)

We can add this behavior when a prefix argument is used (like
C-u C-x p f'), but that's no dynamic switching.

Re: project-find-file: switch to include non-tracked files

[Manuel Uberti]

On 04/05/21 18:43, Dmitry Gutov wrote:
> There are two ways to solve this:
> 
> - Add "whitelisting" syntax to project-vc-ignores and the project-ignores
> method. Then you edit the value of this variable, whitelisting .envrc in the
> given project (or globally), and project-find-file will always include it.
> 
> - We add an arg like NO-PROJECT-IGNORES to the project-files method. Then
> project-find-file could use it in (some?) circumstances.

Both solutions sound good, but considering the prefix argument discussed below,
probably the second one seems like the nicest.

> We can add this behavior when a prefix argument is used (like
> C-u C-x p f'), but that's no dynamic switching.

This would be great. Dynamic switching is not a must-have for me, I was just
offering an example of a possible behaviour. What's important is being able to
get to the ignored file(s) through project-find-file.

-- 
Manuel Uberti
www.manueluberti.eu

Re: project-find-file: switch to include non-tracked files

[Manuel Uberti]

Sorry to bring up this again, but is there anything that can be done on this?

The proposed solution with C-u C-x p f seems very reasonable to me, I don't know 
what others project.el users think though.

-- 
Manuel Uberti
www.manueluberti.eu

Re: project-find-file: switch to include non-tracked files

[Philip Kaludercic]

Manuel Uberti <manuel.uberti@inventati.org> writes:

> Sorry to bring up this again, but is there anything that can be done on this?
>
> The proposed solution with C-u C-x p f seems very reasonable to me, I
> don't know what others project.el users think though.

I agree, my only concern is if there might be a better use for C-u that
we aren't seeing right now.

-- 
	Philip Kaludercic

Re: project-find-file: switch to include non-tracked files

[Stefan Kangas]

Philip Kaludercic <philipk@posteo.net> writes:

> Manuel Uberti <manuel.uberti@inventati.org> writes:
>
> > Sorry to bring up this again, but is there anything that can be done on this?
> >
> > The proposed solution with C-u C-x p f seems very reasonable to me, I
> > don't know what others project.el users think though.
>
> I agree, my only concern is if there might be a better use for C-u that
> we aren't seeing right now.

In projectile, they have:

    (projectile-find-file &optional INVALIDATE-CACHE)

    Jump to a project’s file using completion.
    With a prefix arg INVALIDATE-CACHE invalidates the cache first.

But their default mode of operation is to show all files, not just
tracked ones, so I think they have more use for a cache.

Re: project-find-file: switch to include non-tracked files

[Manuel Uberti]

On 04/10/21 15:44, Stefan Kangas wrote:
> In projectile, they have:
> 
>      (projectile-find-file &optional INVALIDATE-CACHE)
> 
>      Jump to a project’s file using completion.
>      With a prefix arg INVALIDATE-CACHE invalidates the cache first.
> 
> But their default mode of operation is to show all files, not just
> tracked ones, so I think they have more use for a cache.

At the top of project.el I see this:

;; * Reliably cache the list of files in the project, probably using
;;   filenotify.el (if supported) to invalidate.  And avoiding caching
;;   if it's not available (manual cache invalidation is not nice).

So maybe going for manual cache invalidation is not the best approach?

-- 
Manuel Uberti
www.manueluberti.eu

Re: project-find-file: switch to include non-tracked files

[Stefan Kangas]

Manuel Uberti <manuel.uberti@inventati.org> writes:

> At the top of project.el I see this:
>
> ;; * Reliably cache the list of files in the project, probably using
> ;;   filenotify.el (if supported) to invalidate.  And avoiding caching
> ;;   if it's not available (manual cache invalidation is not nice).
>
> So maybe going for manual cache invalidation is not the best approach?

Right, they probably had their reasons for doing that - perhaps it was
just easier - but it sounds like something that we would want to
avoid.

I don't have any other ideas for what to use the prefix key for.
Worst case, we'll need a new command or a double prefix for anything
new that comes up.  So I think using the prefix key here sounds
reasonable - and it also happens to be a very nice user interface,
IMO.

Re: project-find-file: switch to include non-tracked files

[Dmitry Gutov]

On 04.10.2021 17:06, Stefan Kangas wrote:
> Manuel Uberti <manuel.uberti@inventati.org> writes:
> 
>> At the top of project.el I see this:
>>
>> ;; * Reliably cache the list of files in the project, probably using
>> ;;   filenotify.el (if supported) to invalidate.  And avoiding caching
>> ;;   if it's not available (manual cache invalidation is not nice).
>>
>> So maybe going for manual cache invalidation is not the best approach?
> 
> Right, they probably had their reasons for doing that - perhaps it was
> just easier - but it sounds like something that we would want to
> avoid.

Yes, early on I have put it as one of the goals -- to avoid manual cache 
invalidation by the user; that's just not nice. And the user might 
forget to do it, or even not know about this duty (most other editors do 
not require manual cache management like that).

'C-u' seems fine. Now let's decide what kind of behavior we actually 
want from it.

Re: project-find-file: switch to include non-tracked files

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 1831 bytes --]

Hi Manuel,

On 04.10.2021 11:33, Manuel Uberti wrote:
> Sorry to bring up this again, but is there anything that can be done on 
> this?
> 
> The proposed solution with C-u C-x p f seems very reasonable to me, I 
> don't know what others project.el users think though.

It kind of got lost among other issues, sorry. That's doubly easy to do 
with emacs-devel threads, so if you could use Debbugs for feature 
requests in the future, that would be great.

Now, I've whipped up a small POC. See the attachment, try it out.

Since 'find' without ignore instructions is as fast as 'git ls-files' 
(even faster, in my testing, on my machine), it didn't require any 
changes in the API so far.

But is that the behavior we want?

Currently it lists _all_ files in the directory, including, say, all 
contents of .git/ (of which there can be a lot, depending on the 
project, whether it uses 'git flow', etc).

Should we add the common ignores from vc-directory-exclusion-list? To 
simply filter those dirs out?

Maybe something else too? Like grep-find-ignored-files (it lists common 
compiled/object files which one usually doesn't want to search, or even 
visit)?

Combining the vars above would bring the file listing to the default 
'project-ignores' behavior. Which the 'transient' backend uses, for example.

But in the previous iteration of this thread you also referred to Helm's 
'C-c i' behavior. Does it only list the ignored files?

In any case, we could make 'C-u project-find-file' have this behavior: 
listing only ignored files instead. And maybe not all of them: skipping 
the contents of .git/, .bzr/, etc, still sounds useful. The upside is 
possibly having a lot fewer files to choose from.

That *would* require a more involved change in project.el, but at least 
we'd make it after carefully weighing the options.

[-- Attachment #2: project-find-file-no-ignores.diff --]
[-- Type: text/x-patch, Size: 2567 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 3eaa789b3e..15fa86f07c 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -835,28 +835,28 @@ project--read-regexp
                  project-regexp-history-variable)))
 
 ;;;###autoload
-(defun project-find-file ()
+(defun project-find-file (&optional no-ignores)
   "Visit a file (with completion) in the current project.
 
 The filename at point (determined by `thing-at-point'), if any,
 is available as part of \"future history\"."
-  (interactive)
+  (interactive "P")
   (let* ((pr (project-current t))
          (dirs (list (project-root pr))))
-    (project-find-file-in (thing-at-point 'filename) dirs pr)))
+    (project-find-file-in (thing-at-point 'filename) dirs pr no-ignores)))
 
 ;;;###autoload
-(defun project-or-external-find-file ()
+(defun project-or-external-find-file (&optional no-ignores)
   "Visit a file (with completion) in the current project or external roots.
 
 The filename at point (determined by `thing-at-point'), if any,
 is available as part of \"future history\"."
-  (interactive)
+  (interactive "P")
   (let* ((pr (project-current t))
          (dirs (cons
                 (project-root pr)
                 (project-external-roots pr))))
-    (project-find-file-in (thing-at-point 'filename) dirs pr)))
+    (project-find-file-in (thing-at-point 'filename) dirs pr no-ignores)))
 
 (defcustom project-read-file-name-function #'project--read-file-cpd-relative
   "Function to call to read a file name from a list.
@@ -909,12 +909,19 @@ project--read-file-absolute
                                    predicate
                                    hist mb-default))
 
-(defun project-find-file-in (suggested-filename dirs project)
+(defun project-find-file-in (suggested-filename dirs project &optional no-ignores)
   "Complete a file name in DIRS in PROJECT and visit the result.
 
 SUGGESTED-FILENAME is a relative file name, or part of it, which
-is used as part of \"future history\"."
-  (let* ((all-files (project-files project dirs))
+is used as part of \"future history\".
+
+If NO-IGNORES is specified, include all files from DIRS."
+  (let* ((all-files
+          (if no-ignores
+              (mapcan
+               (lambda (dir) (project--files-in-directory dir nil))
+               dirs)
+            (project-files project dirs)))
          (completion-ignore-case read-file-name-completion-ignore-case)
          (file (funcall project-read-file-name-function
                         "Find file" all-files nil nil

Re: project-find-file: switch to include non-tracked files

[Manuel Uberti]

On 05/10/21 21:47, Dmitry Gutov wrote:
> It kind of got lost among other issues, sorry. That's doubly easy to do with 
> emacs-devel threads, so if you could use Debbugs for feature requests in the 
> future, that would be great.

Do you want me to move the discussion on Debbugs?

> Now, I've whipped up a small POC. See the attachment, try it out.

> Since 'find' without ignore instructions is as fast as 'git ls-files' (even 
> faster, in my testing, on my machine), it didn't require any changes in the API 
> so far.

Thank you. I gave it a try and it works as expected.

> But is that the behavior we want?
> 
> Currently it lists _all_ files in the directory, including, say, all contents of 
> .git/ (of which there can be a lot, depending on the project, whether it uses 
> 'git flow', etc).
> 
> Should we add the common ignores from vc-directory-exclusion-list? To simply 
> filter those dirs out?
> 
> Maybe something else too? Like grep-find-ignored-files (it lists common 
> compiled/object files which one usually doesn't want to search, or even visit)?
> 
> Combining the vars above would bring the file listing to the default 
> 'project-ignores' behavior. Which the 'transient' backend uses, for example.

I think ignoring directories such as .git would be good to speed up the command 
and make the candidate list cleaner.

> But in the previous iteration of this thread you also referred to Helm's 'C-c i' 
> behavior. Does it only list the ignored files?

'C-c i' in helm-ls-git toggles the '-o' switch for git ls-files, so it does not 
include the listing of the .git directory in its result.

> In any case, we could make 'C-u project-find-file' have this behavior: listing 
> only ignored files instead. And maybe not all of them: skipping the contents of 
> .git/, .bzr/, etc, still sounds useful. The upside is possibly having a lot 
> fewer files to choose from.

I agree with you.

-- 
Manuel Uberti
www.manueluberti.eu

Re: project-find-file: switch to include non-tracked files

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 1543 bytes --]

On 06.10.2021 08:18, Manuel Uberti wrote:
> On 05/10/21 21:47, Dmitry Gutov wrote:
>> It kind of got lost among other issues, sorry. That's doubly easy to 
>> do with emacs-devel threads, so if you could use Debbugs for feature 
>> requests in the future, that would be great.
> 
> Do you want me to move the discussion on Debbugs?

No, it's fine here now.

> I think ignoring directories such as .git would be good to speed up the 
> command and make the candidate list cleaner.

OK, see the updated patch.

find's performance is really sensitive to the number of ignore entries 
it has to process, so if the difference in performance between two 
invocation types gets too noticeable, while they return approximately 
the same number of entries, customizing vc-directory-exclusion-list to 
have fewer entries can help.

>> But in the previous iteration of this thread you also referred to 
>> Helm's 'C-c i' behavior. Does it only list the ignored files?
> 
> 'C-c i' in helm-ls-git toggles the '-o' switch for git ls-files, so it 
> does not include the listing of the .git directory in its result.

All right, this does seem to include all files, not just the ignored ones.

>> In any case, we could make 'C-u project-find-file' have this behavior: 
>> listing only ignored files instead. And maybe not all of them: 
>> skipping the contents of .git/, .bzr/, etc, still sounds useful. The 
>> upside is possibly having a lot fewer files to choose from.
> 
> I agree with you.

Which of the two behaviors would you like it to have, though?

[-- Attachment #2: project-find-file-no-ignores.diff --]
[-- Type: text/x-patch, Size: 2844 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 3eaa789b3e..65ae43ff4c 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -835,28 +835,28 @@ project--read-regexp
                  project-regexp-history-variable)))
 
 ;;;###autoload
-(defun project-find-file ()
+(defun project-find-file (&optional no-ignores)
   "Visit a file (with completion) in the current project.
 
 The filename at point (determined by `thing-at-point'), if any,
 is available as part of \"future history\"."
-  (interactive)
+  (interactive "P")
   (let* ((pr (project-current t))
          (dirs (list (project-root pr))))
-    (project-find-file-in (thing-at-point 'filename) dirs pr)))
+    (project-find-file-in (thing-at-point 'filename) dirs pr no-ignores)))
 
 ;;;###autoload
-(defun project-or-external-find-file ()
+(defun project-or-external-find-file (&optional no-ignores)
   "Visit a file (with completion) in the current project or external roots.
 
 The filename at point (determined by `thing-at-point'), if any,
 is available as part of \"future history\"."
-  (interactive)
+  (interactive "P")
   (let* ((pr (project-current t))
          (dirs (cons
                 (project-root pr)
                 (project-external-roots pr))))
-    (project-find-file-in (thing-at-point 'filename) dirs pr)))
+    (project-find-file-in (thing-at-point 'filename) dirs pr no-ignores)))
 
 (defcustom project-read-file-name-function #'project--read-file-cpd-relative
   "Function to call to read a file name from a list.
@@ -909,12 +909,24 @@ project--read-file-absolute
                                    predicate
                                    hist mb-default))
 
-(defun project-find-file-in (suggested-filename dirs project)
+(defun project-find-file-in (suggested-filename dirs project &optional no-ignores)
   "Complete a file name in DIRS in PROJECT and visit the result.
 
 SUGGESTED-FILENAME is a relative file name, or part of it, which
-is used as part of \"future history\"."
-  (let* ((all-files (project-files project dirs))
+is used as part of \"future history\".
+
+If NO-IGNORES is specified, include all files from DIRS, except
+for VCS metadata directories enumerated in `vc-directory-exclusion-list'."
+  (let* ((vc-dirs-ignores (mapcar
+                           (lambda (dir)
+                             (concat dir "/"))
+                           vc-directory-exclusion-list))
+         (all-files
+          (if no-ignores
+              (mapcan
+               (lambda (dir) (project--files-in-directory dir vc-dirs-ignores))
+               dirs)
+            (project-files project dirs)))
          (completion-ignore-case read-file-name-completion-ignore-case)
          (file (funcall project-read-file-name-function
                         "Find file" all-files nil nil

Re: project-find-file: switch to include non-tracked files

[Manuel Uberti]

On 06/10/21 08:05, Dmitry Gutov wrote:
> OK, see the updated patch.

Looks good!

> Which of the two behaviors would you like it to have, though?

Sorry, I meant I agree with 'C-u project-find-file' skipping the contents of 
.git/ (.bzr/, etc.) while still displaying ignored files.

-- 
Manuel Uberti
www.manueluberti.eu

Re: project-find-file: switch to include non-tracked files

[Dmitry Gutov]

On 06.10.2021 09:12, Manuel Uberti wrote:
> On 06/10/21 08:05, Dmitry Gutov wrote:
>> OK, see the updated patch.
> 
> Looks good!
> 
>> Which of the two behaviors would you like it to have, though?
> 
> Sorry, I meant I agree with 'C-u project-find-file' skipping the 
> contents of .git/ (.bzr/, etc.) while still displaying ignored files.

Thanks for confirming.

I've pushed the discussed change to master, to be released in Emacs 29 
(but of course you'll be able to install a updated version of project.el 
from ELPA much sooner).

Re: project-find-file: switch to include non-tracked files

[Manuel Uberti]

On 14/10/21 02:47, Dmitry Gutov wrote:
> Thanks for confirming.
> 
> I've pushed the discussed change to master, to be released in Emacs 29 (but of 
> course you'll be able to install a updated version of project.el from ELPA much 
> sooner).

Thank you for the update. On Emacs 29 it works as expected on different projects 
(including Emacs source directory) albeit one, which unfortunately I can share 
much about because the customer owns the code.

I can report the bug if you want, but I will not be able to share a much 
detailed recipe to reproduce the error. From 'emacs -Q' the only thing I did is 
the following:

(dolist (dir '(".clj-kondo" ".cpcache" ".gitlab" "node_modules"))
   (add-to-list 'vc-directory-exclusion-list dir t))

Then C-u C-x p f in the customer's project and I got this:

Debugger entered--Lisp error: (error "File listing failed: 
./shadow-cljs.edn\0./public/cs...")
   error("File listing failed: %s" 
"./shadow-cljs.edn\0./public/css/site.css\0./public/i...")
   project--files-in-directory("~/7bridges/skuro/cockpit/" ("SCCS/" "RCS/" 
"CVS/" "MCVS/" ".src/" ".svn/" ".git/" ".hg/" ".bzr/" "_MTN/" "_darcs/" 
"{arch}/" ".clj-kondo/" ".cpcache/" ".gitlab/" "node_modules/"))
   #f(compiled-function (dir) #<bytecode 
0x16d43be495cb8511>)("~/7bridges/skuro/cockpit/")
   project-find-file-in(nil ("~/7bridges/skuro/cockpit/") (vc . 
"~/7bridges/skuro/cockpit/") (4))
   project-find-file((4))
   funcall-interactively(project-find-file (4))
   command-execute(project-find-file)

Let me know if you want me to file a proper bug.

-- 
Manuel Uberti
www.manueluberti.eu

Re: project-find-file: switch to include non-tracked files

[Dmitry Gutov]

On 14.10.2021 09:37, Manuel Uberti wrote:
> On 14/10/21 02:47, Dmitry Gutov wrote:
>> Thanks for confirming.
>>
>> I've pushed the discussed change to master, to be released in Emacs 29 
>> (but of course you'll be able to install a updated version of 
>> project.el from ELPA much sooner).
> 
> Thank you for the update. On Emacs 29 it works as expected on different 
> projects (including Emacs source directory)

Excellent.

> albeit one, which 
> unfortunately I can share much about because the customer owns the code.
> 
> I can report the bug if you want, but I will not be able to share a much 
> detailed recipe to reproduce the error.

In general, but reports are better, but I don't want to give you the 
run-around. Since reported here, let's continue here.

> From 'emacs -Q' the only thing I 
> did is the following:
> 
> (dolist (dir '(".clj-kondo" ".cpcache" ".gitlab" "node_modules"))
>    (add-to-list 'vc-directory-exclusion-list dir t))

As an aside, I can see what kind of problem this is solving, and this is 
a (meta) problem by itself. We need, like, ignores which are still 
honored when we don't use any other ignore instructions. Maybe a 
separate variable or something? AFAICS helm-ls-git doesn't have anything 
for this.

The above will continue to work until we try to move the listing logic 
to a backend-specific method. Then vc-directory-exclusion-list won't be 
referenced anymore. That's the main reason this feature is not in 
emacs-28 anyway.

Also note that 'find' gets a noticeable slowdown the more ignore entries 
we ask it to handle. Consider removing some entries from 
vc-directory-exclusion-list as well.

> Then C-u C-x p f in the customer's project and I got this:
> 
> Debugger entered--Lisp error: (error "File listing failed: 
> ./shadow-cljs.edn\0./public/cs...")
>    error("File listing failed: %s" 
> "./shadow-cljs.edn\0./public/css/site.css\0./public/i...")
>    project--files-in-directory("~/7bridges/skuro/cockpit/" ("SCCS/" 
> "RCS/" "CVS/" "MCVS/" ".src/" ".svn/" ".git/" ".hg/" ".bzr/" "_MTN/" 
> "_darcs/" "{arch}/" ".clj-kondo/" ".cpcache/" ".gitlab/" "node_modules/"))
>    #f(compiled-function (dir) #<bytecode 
> 0x16d43be495cb8511>)("~/7bridges/skuro/cockpit/")
>    project-find-file-in(nil ("~/7bridges/skuro/cockpit/") (vc . 
> "~/7bridges/skuro/cockpit/") (4))
>    project-find-file((4))
>    funcall-interactively(project-find-file (4))
>    command-execute(project-find-file)
> 
> Let me know if you want me to file a proper bug.

As a rough guess, perhaps some directories in this repo are not readable 
for you?

Try edebug-ing project--files-in-directory. When you reach the (zerop 
status) line, the status will be non-zero for some reason. And there you 
could examine the buffer contents (though they are likely to be quite 
large).

Another approach is to step through the method, then evaluate the 
constructed 'command' (perhaps call (message "%s" command) to make sure 
it's not truncated), then copy it to the terminal with appropriate 
escaping and see the full output and the error status returned. The 
output might include an explanation.

You might want to replace '-print0' with '-print' when running the 
command in the terminal, too, so that it's not just one long line.

Re: project-find-file: switch to include non-tracked files

[Manuel Uberti]

On 14/10/21 14:01, Dmitry Gutov wrote:
> As an aside, I can see what kind of problem this is solving, and this is a 
> (meta) problem by itself. We need, like, ignores which are still honored when we 
> don't use any other ignore instructions. Maybe a separate variable or something? 
> AFAICS helm-ls-git doesn't have anything for this.

> The above will continue to work until we try to move the listing logic to a 
> backend-specific method. Then vc-directory-exclusion-list won't be referenced 
> anymore. That's the main reason this feature is not in emacs-28 anyway.

I didn't know about this.

> Also note that 'find' gets a noticeable slowdown the more ignore entries we ask 
> it to handle. Consider removing some entries from vc-directory-exclusion-list as 
> well.

Interesting, I didn't know about this either, but see below.

> As a rough guess, perhaps some directories in this repo are not readable for you?

Yes, great tip, you nailed it. There is one directory I cannot read in there, so 
I just added that to vc-directory-exclusion-list and everything works as 
expected now.

Thanks a lot!

-- 
Manuel Uberti
www.manueluberti.eu

Re: project-find-file: switch to include non-tracked files

[Dmitry Gutov]

On 14.10.2021 15:06, Manuel Uberti wrote:
> On 14/10/21 14:01, Dmitry Gutov wrote:
>> As an aside, I can see what kind of problem this is solving, and this 
>> is a (meta) problem by itself. We need, like, ignores which are still 
>> honored when we don't use any other ignore instructions. Maybe a 
>> separate variable or something? AFAICS helm-ls-git doesn't have 
>> anything for this.
> 
>> The above will continue to work until we try to move the listing logic 
>> to a backend-specific method. Then vc-directory-exclusion-list won't 
>> be referenced anymore. That's the main reason this feature is not in 
>> emacs-28 anyway.
> 
> I didn't know about this.

If we end up adding a new user option for this, what would you call it?

>> As a rough guess, perhaps some directories in this repo are not 
>> readable for you?
> 
> Yes, great tip, you nailed it. There is one directory I cannot read in 
> there, so I just added that to vc-directory-exclusion-list and 
> everything works as expected now.

We should probably try to handle this is a more transparent way, showing 
the exact error message to the user (when the buffer contains lots of 
output as well, the message becomes next to useless).

Could you try applying the patch below, removing your recent 
customization and seeing whether the error message is better now?

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 79d2e050d9..7c3bb9229d 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -322,7 +322,15 @@ project--files-in-directory
               (process-file-shell-command command nil t))
              (pt (point-min)))
          (unless (zerop status)
-          (error "File listing failed: %s" (buffer-string)))
+          (goto-char (point-min))
+          (if (and
+               (not (eql status 127))
+               (search-forward "Permission denied\n"))
+              (let ((end (1- (point))))
+                (re-search-backward "\\`\\|\0")
+                (error "File listing failed: %s"
+                       (buffer-substring (1+ (point)) end)))
+            (error "File listing failed: %s" (buffer-string))))
          (goto-char pt)
          (while (search-forward "\0" nil t)
            (push (buffer-substring-no-properties (1+ pt) (1- (point)))

Re: project-find-file: switch to include non-tracked files

[Manuel Uberti]

On 14/10/21 23:55, Dmitry Gutov wrote:
> If we end up adding a new user option for this, what would you call it?

Not sure actually. I see a project-ignores and a project-vc-ignores already, 
maybe these can be leveraged instead of adding a new option?

> We should probably try to handle this is a more transparent way, showing the 
> exact error message to the user (when the buffer contains lots of output as 
> well, the message becomes next to useless).
> 
> Could you try applying the patch below, removing your recent customization and 
> seeing whether the error message is better now?
> 
> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
> index 79d2e050d9..7c3bb9229d 100644
> --- a/lisp/progmodes/project.el
> +++ b/lisp/progmodes/project.el
> @@ -322,7 +322,15 @@ project--files-in-directory
>                (process-file-shell-command command nil t))
>               (pt (point-min)))
>           (unless (zerop status)
> -          (error "File listing failed: %s" (buffer-string)))
> +          (goto-char (point-min))
> +          (if (and
> +               (not (eql status 127))
> +               (search-forward "Permission denied\n"))
> +              (let ((end (1- (point))))
> +                (re-search-backward "\\`\\|\0")
> +                (error "File listing failed: %s"
> +                       (buffer-substring (1+ (point)) end)))
> +            (error "File listing failed: %s" (buffer-string))))
>           (goto-char pt)
>           (while (search-forward "\0" nil t)
>             (push (buffer-substring-no-properties (1+ pt) (1- (point)))

I did as you described and I got this message when I did C-u C-x p f:

File listing failed: find: ‘./tmp/db’: Permission denied

Two advantages of your approach:
- the message came up quickly
- it hinted directly to the problem

-- 
Manuel Uberti
www.manueluberti.eu

Re: project-find-file: switch to include non-tracked files

[Dmitry Gutov]

On 15.10.2021 08:24, Manuel Uberti wrote:
> On 14/10/21 23:55, Dmitry Gutov wrote:
>> If we end up adding a new user option for this, what would you call it?
> 
> Not sure actually. I see a project-ignores and a project-vc-ignores 
> already, maybe these can be leveraged instead of adding a new option?

Both of these are for the default project-files behavior.

How would we leverage them?

>> We should probably try to handle this is a more transparent way, 
>> showing the exact error message to the user (when the buffer contains 
>> lots of output as well, the message becomes next to useless).
>>
>> Could you try applying the patch below, removing your recent 
>> customization and seeing whether the error message is better now?
>>
>> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
>> index 79d2e050d9..7c3bb9229d 100644
>> --- a/lisp/progmodes/project.el
>> +++ b/lisp/progmodes/project.el
>> @@ -322,7 +322,15 @@ project--files-in-directory
>>                (process-file-shell-command command nil t))
>>               (pt (point-min)))
>>           (unless (zerop status)
>> -          (error "File listing failed: %s" (buffer-string)))
>> +          (goto-char (point-min))
>> +          (if (and
>> +               (not (eql status 127))
>> +               (search-forward "Permission denied\n"))
>> +              (let ((end (1- (point))))
>> +                (re-search-backward "\\`\\|\0")
>> +                (error "File listing failed: %s"
>> +                       (buffer-substring (1+ (point)) end)))
>> +            (error "File listing failed: %s" (buffer-string))))
>>           (goto-char pt)
>>           (while (search-forward "\0" nil t)
>>             (push (buffer-substring-no-properties (1+ pt) (1- (point)))
> 
> I did as you described and I got this message when I did C-u C-x p f:
> 
> File listing failed: find: ‘./tmp/db’: Permission denied
> 
> Two advantages of your approach:
> - the message came up quickly
> - it hinted directly to the problem

Thanks for checking, I've pushed the patch (slightly tweaked).

Perhaps ideally, we'd just ignore such directories, leaving it to the 
user to figure out why the files are not showing up. But 'find' doesn't 
make it easy. First, adding

   -o -type d -a ! -readable

kinda works, but it's a GNU extension, not available on e.g, macOS, so 
it's a no-go. And even with the '-type d' qualifier it adds some runtime 
cost, somehow.

I suppose we could just redirect stderr to null (like vc-git--out-ok 
does) and check for success by the presence of \0 chars in the output, 
but that's both quite lax and leaves us unable to print the error 
message when indeed some other kind of error happens. I guess 
redirecting stderr to a file is the remaining option...

Re: project-find-file: switch to include non-tracked files

[Manuel Uberti]

On 15/10/21 14:12, Dmitry Gutov wrote:
> Both of these are for the default project-files behavior.
> 
> How would we leverage them?

Not sure, sorry. I don't know the rationale behind the decision to stop using 
vc-directory-exclusion-list, and I have to dig deeper in project.el to better 
understand what project-ignores and a project-vc-ignores do and how they are 
used at the moment.

> Thanks for checking, I've pushed the patch (slightly tweaked).

I'll give it a try later and come back if I find any issue with it.

> Perhaps ideally, we'd just ignore such directories, leaving it to the user to 
> figure out why the files are not showing up. But 'find' doesn't make it easy. 
> First, adding
> 
>    -o -type d -a ! -readable
> 
> kinda works, but it's a GNU extension, not available on e.g, macOS, so it's a 
> no-go. And even with the '-type d' qualifier it adds some runtime cost, somehow.
> 
> I suppose we could just redirect stderr to null (like vc-git--out-ok does) and 
> check for success by the presence of \0 chars in the output, but that's both 
> quite lax and leaves us unable to print the error message when indeed some other 
> kind of error happens. I guess redirecting stderr to a file is the remaining 
> option...

I am ok with customizing a setting to ignore specific directories, but as you 
said before it should not slow down find. But parsing \0 chars sound a bit scary 
to me, so yeah, it's a tricky one.

-- 
Manuel Uberti
www.manueluberti.eu

Re: project-find-file: switch to include non-tracked files

[Dmitry Gutov]

On 15.10.2021 16:05, Manuel Uberti wrote:
> On 15/10/21 14:12, Dmitry Gutov wrote:
>> Both of these are for the default project-files behavior.
>>
>> How would we leverage them?
> 
> Not sure, sorry. I don't know the rationale behind the decision to stop 
> using vc-directory-exclusion-list,

For performance, for better semantics. If we delegate this file listing 
to backends as well (they can often do it faster, or with better 
adherence to user's intent), using a variable starting with vc- will not 
make sense.

> and I have to dig deeper in 
> project.el to better understand what project-ignores and a 
> project-vc-ignores do and how they are used at the moment.

There's no need to get deep into the implementation. project-vc-ignores 
affects what 'M-x project-find-file' does. By specifying ignore rules.

'C-u M-x project-file-file' essentially differs from the version without 
prefix by using a different set of ignore rules. So it can't really use 
the same variable, right?

Anyway, it's not urgent. Ideas welcome anytime.

>> Thanks for checking, I've pushed the patch (slightly tweaked).
> 
> I'll give it a try later and come back if I find any issue with it.

Thanks!