bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Juri Linkov]

Severity: wishlist

For example, typing 'C-x v L' (vc-print-root-log) outside of a project
displays the minibuffer with the prompt "Directory for VC revision log: ".
It should provide a list of the default values from the project list
saved in ~/.emacs.d/projects

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 12.06.2020 02:03, Juri Linkov wrote:
> For example, typing 'C-x v L' (vc-print-root-log) outside of a project
> displays the minibuffer with the prompt "Directory for VC revision log: ".
> It should provide a list of the default values from the project list
> saved in ~/.emacs.d/projects

Note that one would have to verify that each directory in the list is a 
part of a VC repository.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Juri Linkov]

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

>> For example, typing 'C-x v L' (vc-print-root-log) outside of a project
>> displays the minibuffer with the prompt "Directory for VC revision log: ".
>> It should provide a list of the default values from the project list
>> saved in ~/.emacs.d/projects
>
> Note that one would have to verify that each directory in the list is
> a part of a VC repository.

Maybe something like:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: project-vc-list.patch --]
[-- Type: text/x-diff, Size: 1136 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 7a41df614b..fcba4136b0 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -897,6 +897,12 @@ project-prompt-project-dir
         (read-directory-name "Select directory: " default-directory nil t)
       pr-dir)))
 
+;;;###autoload
+(defun project-vc-list ()
+  "Return a list of VC-based projects."
+  (project--ensure-read-project-list)
+  (seq-filter #'project-try-vc (mapcar #'car project--list)))
+
 \f
 ;;; Project switching
 
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 08b1a85c9b..a640067de1 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -2559,7 +2559,8 @@ vc-print-root-log
 	 rootdir)
     (if backend
 	(setq rootdir (vc-call-backend backend 'root default-directory))
-      (setq rootdir (read-directory-name "Directory for VC revision log: "))
+      (setq rootdir (read-directory-name "Directory for VC revision log: " nil
+                                         (project-vc-list)))
       (setq backend (vc-responsible-backend rootdir))
       (unless backend
         (error "Directory is not version controlled")))

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 19.06.2020 02:28, Juri Linkov wrote:
> Maybe something like:

Not too bad.

Though I'd rather not extend the public contract of project.el with a 
function that special-cases VC projects.

So maybe something like this instead:

diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 9b12d44978..19a4c7f60d 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -2508,6 +2508,19 @@ vc-print-log
           )
      (vc-print-log-internal backend files working-revision nil limit)))

+;; Or use project-try-vc after all. But this should be faster in the
+;; event when there actually are non-VC based projects in the list.
+(defun vc--known-vc-roots ()
+  (require 'project)
+  (defvar project--list)
+  (project--ensure-read-project-list)
+  (seq-filter (lambda (dir)
+                (let ((backend (vc-responsible-backend dir)))
+                  (if backend
+                      (condition-case nil
+                          (vc-call-backend backend 'root dir)
+                        (vc-not-supported
+                         nil)))))
+              (mapcar #'car project--list)))
+
  ;;;###autoload
  (defun vc-print-root-log (&optional limit revision)
    "List the revision history for the current VC controlled tree in a 
window.
@@ -2547,7 +2560,9 @@ vc-print-root-log
  	 rootdir)
      (if backend
  	(setq rootdir (vc-call-backend backend 'root default-directory))
-      (setq rootdir (read-directory-name "Directory for VC revision 
log: "))
+      (setq rootdir (read-directory-name "Directory for VC revision log: "
+                                         nil
+                                         (vc--known-vc-roots)))
        (setq backend (vc-responsible-backend rootdir))
        (unless backend
          (error "Directory is not version controlled")))

Personally, though, when I want behavior like this, I would probably 
just type 'C-x p v'.

The directory name reading with completion performed by 
project-prompt-project-dir is more quick and handy (though I'll confess 
to using Ivy as the completion UI for this and one other function; 
vertical completion fits these long string values best).

It also puts the selected project on the top of the list, which 
vc--known-vc-roots (or your function) don't do.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Juri Linkov]

>> Maybe something like:
>
> Not too bad.
>
> Though I'd rather not extend the public contract of project.el with
> a function that special-cases VC projects.
>
> So maybe something like this instead:
>
> +;; Or use project-try-vc after all. But this should be faster in the
> +;; event when there actually are non-VC based projects in the list.
> +(defun vc--known-vc-roots ()
> +  (require 'project)
> +  (defvar project--list)
> +  (project--ensure-read-project-list)

Calling internal project.el functions from vc.el?  Really?

> Personally, though, when I want behavior like this, I would probably just
> type 'C-x p v'.

'C-x p v' is not a replacement for 'C-x v L'.

> The directory name reading with completion performed by
> project-prompt-project-dir is more quick and handy (though I'll confess 
> to using Ivy as the completion UI for this and one other function; vertical
> completion fits these long string values best).

'M-n' works fine without Ivy to select a recent project dir.

> It also puts the selected project on the top of the list, which
> vc--known-vc-roots (or your function) don't do.

I don't understand what is the selected project.  The current project?
Then neither 'C-x p v', nor 'C-x v L' should ask for a project directory
when called from default-directory of the current project.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 21.06.2020 02:51, Juri Linkov wrote:
>>> Maybe something like:
>>
>> Not too bad.
>>
>> Though I'd rather not extend the public contract of project.el with
>> a function that special-cases VC projects.
>>
>> So maybe something like this instead:
>>
>> +;; Or use project-try-vc after all. But this should be faster in the
>> +;; event when there actually are non-VC based projects in the list.
>> +(defun vc--known-vc-roots ()
>> +  (require 'project)
>> +  (defvar project--list)
>> +  (project--ensure-read-project-list)
> 
> Calling internal project.el functions from vc.el?  Really?

The alternative would be to create a function like that in project.el.

But I hesitate to make it public, sorry.

>> Personally, though, when I want behavior like this, I would probably just
>> type 'C-x p v'.
> 
> 'C-x p v' is not a replacement for 'C-x v L'.

'C-x p v L', then?

>> The directory name reading with completion performed by
>> project-prompt-project-dir is more quick and handy (though I'll confess
>> to using Ivy as the completion UI for this and one other function; vertical
>> completion fits these long string values best).
> 
> 'M-n' works fine without Ivy to select a recent project dir.

Just saying what works better for me.

>> It also puts the selected project on the top of the list, which
>> vc--known-vc-roots (or your function) don't do.
> 
> I don't understand what is the selected project.  The current project?

When you select a project to use, the prompting subroutine 
(project-prompt-project-dir) re-sorts the saved list of project. It is 
somewhat of an argument to only go through this UI when one needs to 
select a project to use.

> Then neither 'C-x p v', nor 'C-x v L' should ask for a project directory
> when called from default-directory of the current project.

Not sure what you mean. I think they don't? When called inside a project.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Juri Linkov]

>>> +;; Or use project-try-vc after all. But this should be faster in the
>>> +;; event when there actually are non-VC based projects in the list.
>>> +(defun vc--known-vc-roots ()
>>> +  (require 'project)
>>> +  (defvar project--list)
>>> +  (project--ensure-read-project-list)
>>
>> Calling internal project.el functions from vc.el?  Really?
>
> The alternative would be to create a function like that in project.el.
>
> But I hesitate to make it public, sorry.

Why not?

We have to decide which of them should be dependent.  It would be fine
either way: project.el to use vc.el public functions, or vice versa.

>>> Personally, though, when I want behavior like this, I would probably just
>>> type 'C-x p v'.
>> 'C-x p v' is not a replacement for 'C-x v L'.
>
> 'C-x p v L', then?

'C-x v L' needs to provide a list of recently used repositories.
Actually, this feature doesn't depend on project.el, so recently
used repositories could be recorded independently without project.el.
But it would be nice to share these directories between project.el and vc.el.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 22.06.2020 01:49, Juri Linkov wrote:
>>>> +;; Or use project-try-vc after all. But this should be faster in the
>>>> +;; event when there actually are non-VC based projects in the list.
>>>> +(defun vc--known-vc-roots ()
>>>> +  (require 'project)
>>>> +  (defvar project--list)
>>>> +  (project--ensure-read-project-list)
>>>
>>> Calling internal project.el functions from vc.el?  Really?
>>
>> The alternative would be to create a function like that in project.el.
>>
>> But I hesitate to make it public, sorry.
> 
> Why not?

Because I'd rather not have external code depend on what exact kind of 
project backend is in use.

> We have to decide which of them should be dependent.  It would be fine
> either way: project.el to use vc.el public functions, or vice versa.

There are other private functions shared between project.el and xref.el. 
So I this is fine, kind of.

But you have noted an alternative option below:

>>>> Personally, though, when I want behavior like this, I would probably just
>>>> type 'C-x p v'.
>>> 'C-x p v' is not a replacement for 'C-x v L'.
>>
>> 'C-x p v L', then?
> 
> 'C-x v L' needs to provide a list of recently used repositories.

If you like.

> Actually, this feature doesn't depend on project.el, so recently
> used repositories could be recorded independently without project.el.

This is the alternative option. It would create a yet another file in 
the user's dir, though.

> But it would be nice to share these directories between project.el and vc.el.

Perhaps there could be a public function in project.el called 
project-known-roots.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Juri Linkov]

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

>> But it would be nice to share these directories between project.el and vc.el.
>
> Perhaps there could be a public function in project.el called
> project-known-roots.

Like this?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: project-known-roots.patch --]
[-- Type: text/x-diff, Size: 1310 bytes --]

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 2f213dab8b..f489145e92 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -935,6 +935,12 @@ project-prompt-project-dir
         (read-directory-name "Select directory: " default-directory nil t)
       pr-dir)))
 
+;;;###autoload
+(defun project-known-roots ()
+  "Return a list of known roots."
+  (project--ensure-read-project-list)
+  (mapcar #'project-try-vc (mapcar #'car project--list)))
+
 \f
 ;;; Project switching
 
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 9b12d44978..cc83d9a7a1 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -2547,7 +2555,10 @@ vc-print-root-log
 	 rootdir)
     (if backend
 	(setq rootdir (vc-call-backend backend 'root default-directory))
-      (setq rootdir (read-directory-name "Directory for VC revision log: "))
+      (setq rootdir (read-directory-name "Directory for VC revision log: " nil
+                                         (mapcan (lambda (r) (when (eq (car r) 'vc)
+                                                               (list (cdr r))))
+                                                 (project-known-roots))))
       (setq backend (vc-responsible-backend rootdir))
       (unless backend
         (error "Directory is not version controlled")))

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 23.06.2020 02:45, Juri Linkov wrote:
> Like this?

Close.

But without mapping through project-try-vc. The function will return 
_all_ known roots, in VC repositories or not.

vc-print-root-log can call it with seq-filter (project-try-vc is a 
public function). No need to deconstruct the returned values either.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Juri Linkov]

tags 41821 fixed
close 41821 28.0.50
quit

> Close.

I interpreted this as a verb in the imperative mood, so closed the report :)

> But without mapping through project-try-vc. The function will return _all_
> known roots, in VC repositories or not.
>
> vc-print-root-log can call it with seq-filter (project-try-vc is a public
> function). No need to deconstruct the returned values either.

Now pushed to master with these fixes.  Thanks for the help.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Wed, 24 Jun 2020 02:59:49 +0300
> Cc: 41821@debbugs.gnu.org
> 
> I interpreted this as a verb in the imperative mood, so closed the report :)
> 
> > But without mapping through project-try-vc. The function will return _all_
> > known roots, in VC repositories or not.
> >
> > vc-print-root-log can call it with seq-filter (project-try-vc is a public
> > function). No need to deconstruct the returned values either.
> 
> Now pushed to master with these fixes.  Thanks for the help.

This changeset causes byte-compilation warnings:

  In end of data:
  vc/vc-hooks.el:1013:1: Warning: the function `project-known-roots' is not
      known to be defined.

More importantly, this is not very clean, IMO: vc-hooks.el is
preloaded, so any attempt to call a project.el function in it runs a
very real risk to force loading project.el, something that we didn't
yet decide to do.

Why does this have to be in vc-hooks.el, and if it has to be there,
can it please not call functions that re not preloaded?

Thanks.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Basil L. Contovounesios]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Juri Linkov <juri@linkov.net>
>> Date: Wed, 24 Jun 2020 02:59:49 +0300
>> Cc: 41821@debbugs.gnu.org
>> 
>> Now pushed to master with these fixes.  Thanks for the help.
>
> This changeset causes byte-compilation warnings:
>
>   In end of data:
>   vc/vc-hooks.el:1013:1: Warning: the function `project-known-roots' is not
>       known to be defined.

Since project-known-roots is autoloaded, will this go away when
ldefs-boot.el is updated?

-- 
Basil

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 24.06.2020 17:39, Eli Zaretskii wrote:
> vc-hooks.el is
> preloaded, so any attempt to call a project.el function in it runs a
> very real risk to force loading project.el, something that we didn't
> yet decide to do.

I don't quite understand. Is it a problem to call a non-preloded 
function at runtime?

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: Juri Linkov <juri@linkov.net>,  41821@debbugs.gnu.org,  dgutov@yandex.ru
> Date: Wed, 24 Jun 2020 15:52:20 +0100
> 
> >   In end of data:
> >   vc/vc-hooks.el:1013:1: Warning: the function `project-known-roots' is not
> >       known to be defined.
> 
> Since project-known-roots is autoloaded, will this go away when
> ldefs-boot.el is updated?

You mean loaddefs.el, I presume (ldefs-boot is rarely used during the
build).

Probably, but that's not my main problem with that change.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Eli Zaretskii]

> Cc: 41821@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 24 Jun 2020 18:10:52 +0300
> 
> On 24.06.2020 17:39, Eli Zaretskii wrote:
> > vc-hooks.el is
> > preloaded, so any attempt to call a project.el function in it runs a
> > very real risk to force loading project.el, something that we didn't
> > yet decide to do.
> 
> I don't quite understand. Is it a problem to call a non-preloded 
> function at runtime?

As I tried to explain, I'd like to avoid calling such functions from
preloaded code.

Is that possible, please?

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Eli Zaretskii]

> Date: Wed, 24 Jun 2020 18:18:10 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 41821@debbugs.gnu.org, juri@linkov.net
> 
> As I tried to explain, I'd like to avoid calling such functions from
> preloaded code.

Maybe I should have said explicitly something I thought was quite
obvious: a file FOO-hooks.el is usually a preloaded part of package
FOO.  It is therefore unusual, even IMO somewhat unclean, to have
FOO-hooks.el to call non-preloaded functions from package other than
FOO.

So is it possible to have vc-known-roots defined in vc.el instead of
in vc-hooks.el?

TIA

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 24.06.2020 18:42, Eli Zaretskii wrote:
> So is it possible to have vc-known-roots defined in vc.el instead of
> in vc-hooks.el?

Okay, but that new function delegates to code in project.el anyway, what 
would be the practical difference?

OTOH, if you just said we shouldn't use project.el in vc/*, I could 
understand that.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Eli Zaretskii]

> Cc: 41821@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 24 Jun 2020 21:13:07 +0300
> 
> On 24.06.2020 18:42, Eli Zaretskii wrote:
> > So is it possible to have vc-known-roots defined in vc.el instead of
> > in vc-hooks.el?
> 
> Okay, but that new function delegates to code in project.el anyway, what 
> would be the practical difference?

It would be somewhat cleaner, I think.  Doesn't vc.el already have
some calls to project.el?

> OTOH, if you just said we shouldn't use project.el in vc/*, I could 
> understand that.

That'd be unreasonably harsh, I think.

Actually, I have a question: isn't project.el conceptually a
higher-level feature than VC?  If so, how come VC wants to call
project.el?

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 24.06.2020 02:59, Juri Linkov wrote:

>> Close.
> 
> I interpreted this as a verb in the imperative mood, so closed the report :)

A bit premature: both the name and the docstring need to be changed. And 
the return value currently returns a private structure, instead of a 
list of strings, as promised by the name.

But since we might revert this commit either way, let's shelve 
discussing these details for now.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 24.06.2020 21:29, Eli Zaretskii wrote:
>> Cc: 41821@debbugs.gnu.org, juri@linkov.net
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Wed, 24 Jun 2020 21:13:07 +0300
>>
>> On 24.06.2020 18:42, Eli Zaretskii wrote:
>>> So is it possible to have vc-known-roots defined in vc.el instead of
>>> in vc-hooks.el?
>>
>> Okay, but that new function delegates to code in project.el anyway, what
>> would be the practical difference?
> 
> It would be somewhat cleaner, I think.

That's the whole reason? I mean, I'm not going to protest against an 
extra wrapper, but that doesn't sound like it would solve any practical 
problems. "Cleaner" solutions often have those.

> Doesn't vc.el already have some calls to project.el?

Nope.

>> OTOH, if you just said we shouldn't use project.el in vc/*, I could
>> understand that.
> 
> That'd be unreasonably harsh, I think.

But that would be a limitation I could understand (don't use 
non-preloaded code from preloaded code, period).

> Actually, I have a question: isn't project.el conceptually a
> higher-level feature than VC?  If so, how come VC wants to call
> project.el?

VC doesn't serve project.el only. project.el doesn't solely use VC.

Apparently Juri wants to use certain data collected and saved by 
project.el UI, for convenience.

The alternative would be to introduce some separate history-keeping 
feature for the cases when VC code needs to ask the user to point to a 
VC repository.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Juri Linkov]

> So is it possible to have vc-known-roots defined in vc.el instead of
> in vc-hooks.el?

Actually I see no problem with vc-known-roots defined in vc-hooks.el,
but moving it to vc.el should be fine too: all vc subpackages require vc.el,
so vc-known-roots will be available in these backends from vc.el.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Juri Linkov]

> Apparently Juri wants to use certain data collected and saved by project.el
> UI, for convenience.

This feature was implemented to address Stephen's concerns in
https://debbugs.gnu.org/38044#233

> The alternative would be to introduce some separate history-keeping feature
> for the cases when VC code needs to ask the user to point to
> a VC repository.

It wouldn't be rational to duplicate this feature.
Rather I think the project-list should be updated
even on using vc commands such as 'C-x v d', i.e.
project and vc should be more tightly integrated.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 25.06.2020 02:25, Juri Linkov wrote:
>> The alternative would be to introduce some separate history-keeping feature
>> for the cases when VC code needs to ask the user to point to
>> a VC repository.
> It wouldn't be rational to duplicate this feature.
> Rather I think the project-list should be updated
> even on using vc commands such as 'C-x v d', i.e.
> project and vc should be more tightly integrated.

That would mean opening the project history storage to outside packages. 
I don't know.

To be honest, I almost never experience the problem this bug was fixing. 
Before requesting revision log, I'm almost always already in some 
project buffer.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Eli Zaretskii]

> Cc: 41821@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 24 Jun 2020 21:44:01 +0300
> 
> >> Okay, but that new function delegates to code in project.el anyway, what
> >> would be the practical difference?
> > 
> > It would be somewhat cleaner, I think.
> 
> That's the whole reason?

Well, the whole issue at hand is a rather minor one.

> I mean, I'm not going to protest against an 
> extra wrapper, but that doesn't sound like it would solve any practical 
> problems. "Cleaner" solutions often have those.

In general, code that doesn't _have_ to be preloaded, shouldn't be.
If nothing else, it keeps the memory footprint of a bare Emacs
smaller, and thus prevents us from slipping down the slippery slope of
memory bloat.

> > Actually, I have a question: isn't project.el conceptually a
> > higher-level feature than VC?  If so, how come VC wants to call
> > project.el?
> 
> VC doesn't serve project.el only. project.el doesn't solely use VC.

Yes, but that's not what I asked.  I have the impression that
project.el builds on VC as one project back-end, so it sounds strange
to me that VC turns around and calls project.el for something.

> Apparently Juri wants to use certain data collected and saved by 
> project.el UI, for convenience.

After reading the original complaint that Juri says he wanted to
resolve, I still don't understand why we use project.el for that.  No
one says that every relevant VC repository must have been visited as a
project as part of the current Emacs session.  Why not have some
relevant history in VC itself?

> The alternative would be to introduce some separate history-keeping 
> feature for the cases when VC code needs to ask the user to point to a 
> VC repository.

Exactly.  Why not?

Juri answers:

> It wouldn't be rational to duplicate this feature.
> Rather I think the project-list should be updated
> even on using vc commands such as 'C-x v d', i.e.
> project and vc should be more tightly integrated.

I don't think I agree.  First, I don't see any duplication here:
people could (and do) use VC directly, not through project.el, in
which case project.el history wouldn't help.  Moreover, using a
command such as "C-x v d" doesn't mean I want project.el record that,
again because I might be using project.el commands for projects that
aren't based on VC.

And if the history is collected by VC, it could be made available to
project.el commands that call into VC, right?

Anyway, if you-two feel strongly about keeping the current solution,
i.e. having VC commands use project.el-collected history, I'd
appreciate if that function could be moved to vc.el from vc-hooks.el,
thanks in advance.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 25.06.2020 16:20, Eli Zaretskii wrote:

>> I mean, I'm not going to protest against an
>> extra wrapper, but that doesn't sound like it would solve any practical
>> problems. "Cleaner" solutions often have those.
> 
> In general, code that doesn't _have_ to be preloaded, shouldn't be.
> If nothing else, it keeps the memory footprint of a bare Emacs
> smaller, and thus prevents us from slipping down the slippery slope of
> memory bloat.

And having vc-hooks call project.el functions at runtime would somehow 
force us to preload it? How?

>>> Actually, I have a question: isn't project.el conceptually a
>>> higher-level feature than VC?  If so, how come VC wants to call
>>> project.el?
>>
>> VC doesn't serve project.el only. project.el doesn't solely use VC.
> 
> Yes, but that's not what I asked.  I have the impression that
> project.el builds on VC as one project back-end, so it sounds strange
> to me that VC turns around and calls project.el for something.

Considering one doesn't exclusively serve the other, I wouldn't say 
there is a strict hierarchy.

To be more accurate, we're actually talking about different parts of VC 
and project.el (the UI and infrastructure parts), which have different 
relations.

>> Apparently Juri wants to use certain data collected and saved by
>> project.el UI, for convenience.
> 
> After reading the original complaint that Juri says he wanted to
> resolve, I still don't understand why we use project.el for that.  No
> one says that every relevant VC repository must have been visited as a
> project as part of the current Emacs session.  Why not have some
> relevant history in VC itself?

I would be totally fine with that solution as well.

>> The alternative would be to introduce some separate history-keeping
>> feature for the cases when VC code needs to ask the user to point to a
>> VC repository.
> 
> Exactly.  Why not?

The downside, of course, is having the user input the same thing 
multiple times sometimes. And some extra code.

Overall, both seem minor (and the inconvenience is going to be infrequent).

> And if the history is collected by VC, it could be made available to
> project.el commands that call into VC, right?

But we want to store history on all projects, not just VC based ones.

> Anyway, if you-two feel strongly about keeping the current solution,
> i.e. having VC commands use project.el-collected history, I'd
> appreciate if that function could be moved to vc.el from vc-hooks.el,
> thanks in advance.

We can't just move it: it accesses information private to project.el. 
The best we could do is a wrapper function.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Eli Zaretskii]

> Cc: 41821@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 25 Jun 2020 16:50:24 +0300
> 
> > In general, code that doesn't _have_ to be preloaded, shouldn't be.
> > If nothing else, it keeps the memory footprint of a bare Emacs
> > smaller, and thus prevents us from slipping down the slippery slope of
> > memory bloat.
> 
> And having vc-hooks call project.el functions at runtime would somehow 
> force us to preload it? How?

It's enough that someone makes the function a macro, or does something
else similarly innocent.

> > Anyway, if you-two feel strongly about keeping the current solution,
> > i.e. having VC commands use project.el-collected history, I'd
> > appreciate if that function could be moved to vc.el from vc-hooks.el,
> > thanks in advance.
> 
> We can't just move it: it accesses information private to project.el. 
> The best we could do is a wrapper function.

Sorry, I don't understand: why can't we move it from vc-hooks.el to
vc.el, and why doing that would need a wrapper?

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 25.06.2020 19:31, Eli Zaretskii wrote:

>> And having vc-hooks call project.el functions at runtime would somehow
>> force us to preload it? How?
> 
> It's enough that someone makes the function a macro, or does something
> else similarly innocent.

Nobody will make it a macro, or a defsubst. And I wouldn't say making it 
a macro would be "innocent".

>>> Anyway, if you-two feel strongly about keeping the current solution,
>>> i.e. having VC commands use project.el-collected history, I'd
>>> appreciate if that function could be moved to vc.el from vc-hooks.el,
>>> thanks in advance.
>>
>> We can't just move it: it accesses information private to project.el.
>> The best we could do is a wrapper function.
> 
> Sorry, I don't understand: why can't we move it from vc-hooks.el to
> vc.el, and why doing that would need a wrapper?

It would be a wrapper for a project.el function. But if it works for 
you, okay.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Eli Zaretskii]

> Cc: 41821@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 25 Jun 2020 19:45:45 +0300
> 
> >>> Anyway, if you-two feel strongly about keeping the current solution,
> >>> i.e. having VC commands use project.el-collected history, I'd
> >>> appreciate if that function could be moved to vc.el from vc-hooks.el,
> >>> thanks in advance.
> >>
> >> We can't just move it: it accesses information private to project.el.
> >> The best we could do is a wrapper function.
> > 
> > Sorry, I don't understand: why can't we move it from vc-hooks.el to
> > vc.el, and why doing that would need a wrapper?
> 
> It would be a wrapper for a project.el function. But if it works for 
> you, okay.

We are going in circles again.  Can you explain why vc-known-roots
cannot be moved to vc.el in its entirety?

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 25.06.2020 20:09, Eli Zaretskii wrote:

>> It would be a wrapper for a project.el function. But if it works for
>> you, okay.
> 
> We are going in circles again.  Can you explain why vc-known-roots
> cannot be moved to vc.el in its entirety?

To re-iterate:

- We can't just move it: it accesses information private to project.el.

- We can't move said information (the variable and mechanism to 
save/restore it) because, for one thing, we want to store history on all 
projects, not just VC based ones.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Eli Zaretskii]

> Cc: 41821@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 25 Jun 2020 20:19:27 +0300
> 
> > We are going in circles again.  Can you explain why vc-known-roots
> > cannot be moved to vc.el in its entirety?
> 
> To re-iterate:
> 
> - We can't just move it: it accesses information private to project.el.

What does project.el has to do with this?  I didn't ask to move the
function to project.el.  What am I missing?

> - We can't move said information (the variable and mechanism to 
> save/restore it)

Which information? what variable?  Please be more specific.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 25.06.2020 20:45, Eli Zaretskii wrote:
>> To re-iterate:
>>
>> - We can't just move it: it accesses information private to project.el.
> What does project.el has to do with this?  I didn't ask to move the
> function to project.el.  What am I missing?

You asked to move it *from* project.el.

If that sounds confusing, perhaps try to answer this question: which 
function do you want to see moved?

>> - We can't move said information (the variable and mechanism to
>> save/restore it)
> Which information? what variable?  Please be more specific.

project--list.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Eli Zaretskii]

> Cc: 41821@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 25 Jun 2020 20:50:47 +0300
> 
> On 25.06.2020 20:45, Eli Zaretskii wrote:
> >> To re-iterate:
> >>
> >> - We can't just move it: it accesses information private to project.el.
> > What does project.el has to do with this?  I didn't ask to move the
> > function to project.el.  What am I missing?
> 
> You asked to move it *from* project.el.

This is a huge misunderstanding.

> If that sounds confusing, perhaps try to answer this question: which 
> function do you want to see moved?

As I said before: I'm talking about moving vc-known-roots from
vc-hooks.el to vc.el.  Quoting myself:

> > We are going in circles again.  Can you explain why vc-known-roots
> > cannot be moved to vc.el in its entirety?

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 25.06.2020 21:03, Eli Zaretskii wrote:

>> If that sounds confusing, perhaps try to answer this question: which
>> function do you want to see moved?
> 
> As I said before: I'm talking about moving vc-known-roots from
> vc-hooks.el to vc.el.  Quoting myself:
> 
>>> We are going in circles again.  Can you explain why vc-known-roots
>>> cannot be moved to vc.el in its entirety?

Okay.

As I said, it will be (remain) a wrapper for project.el functions. If 
that's okay with you, we can go ahead.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Eli Zaretskii]

> Cc: 41821@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 25 Jun 2020 21:13:13 +0300
> 
> >>> We are going in circles again.  Can you explain why vc-known-roots
> >>> cannot be moved to vc.el in its entirety?
> 
> Okay.
> 
> As I said, it will be (remain) a wrapper for project.el functions. If 
> that's okay with you, we can go ahead.

You mean, vc-known-roots is a wrapper for project-known-roots?  It was
okay with you to have that, so it's okay with me.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Juri Linkov]

> both the name and the docstring need to be changed

I can't believe that you renamed project-known-roots to
project-known-project-roots.  Why duplicate prefixes 'project-'?

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Juri Linkov]

> Why not have some relevant history in VC itself?

Then like the project history currently is saved in ~/.emacs.d/projects
saving of a separate vc-directory history will be duplicated the same way
in e.g. ~/.emacs.d/vc-projects?

> Moreover, using a command such as "C-x v d" doesn't mean I want
> project.el record that, again because I might be using project.el
> commands for projects that aren't based on VC.

Then project.el could have a customizable variable that will prevent
recording vc project history.

> And if the history is collected by VC, it could be made available to
> project.el commands that call into VC, right?

You mean to record an internal vc-history separately from project.el,
but still to save it in the same project file ~/.emacs.d/projects?

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 28.06.2020 02:44, Juri Linkov wrote:
>> both the name and the docstring need to be changed
> I can't believe that you renamed project-known-roots to
> project-known-project-roots.  Why duplicate prefixes 'project-'?

To emphasize that the roots belong to different projects, and not the 
current one.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Dmitry Gutov <dgutov@yandex.ru>,  41821@debbugs.gnu.org
> Date: Sun, 28 Jun 2020 02:51:06 +0300
> 
> > Why not have some relevant history in VC itself?
> 
> Then like the project history currently is saved in ~/.emacs.d/projects
> saving of a separate vc-directory history will be duplicated the same way
> in e.g. ~/.emacs.d/vc-projects?

Not necessarily duplicated, because no one said the projects I use
will necessarily visit all (indeed, even one of) the VC roots that I
will visit via VC commands.  And vice versa.

IOW, please don't assume that, because project.el can use the VC
backend, it will always use VC in the same directories.  If the roots
are different, offering the same completion candidates will be
confusing and even annoying.

> > Moreover, using a command such as "C-x v d" doesn't mean I want
> > project.el record that, again because I might be using project.el
> > commands for projects that aren't based on VC.
> 
> Then project.el could have a customizable variable that will prevent
> recording vc project history.

What for? let every layer record its own history, and then use the
relevant history in each layer.

> > And if the history is collected by VC, it could be made available to
> > project.el commands that call into VC, right?
> 
> You mean to record an internal vc-history separately from project.el,
> but still to save it in the same project file ~/.emacs.d/projects?

No, I mean provide a possibility for project.el to access the history
saved in ~/.emacs.d/vc-history, in addition to project.el's own
history.  If the user wants that.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Juri Linkov]

>>> both the name and the docstring need to be changed
>> I can't believe that you renamed project-known-roots to
>> project-known-project-roots.  Why duplicate prefixes 'project-'?
>
> To emphasize that the roots belong to different projects, and not the
> current one.

Oh, I see that project can have several roots with project-roots and
project-external-roots.  But still would it be possible to deuglify the
name project-known-project-roots?  Maybe project-all-known-roots?

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Juri Linkov]

>> > Actually, I have a question: isn't project.el conceptually a
>> > higher-level feature than VC?  If so, how come VC wants to call
>> > project.el?
>>
>> VC doesn't serve project.el only. project.el doesn't solely use VC.
>
> Yes, but that's not what I asked.  I have the impression that
> project.el builds on VC as one project back-end, so it sounds strange
> to me that VC turns around and calls project.el for something.

By analogy, vc-git.el is one of back-ends of vc.el, so in vc-git.el
there is such line for using vc functions:

  (require 'vc)

So analogously, vc.el is one of back-ends of project.el, so in vc.el
there should be such line for using project functions:

  (require 'project)

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Juri Linkov]

>> > Why not have some relevant history in VC itself?
>>
>> Then like the project history currently is saved in ~/.emacs.d/projects
>> saving of a separate vc-directory history will be duplicated the same way
>> in e.g. ~/.emacs.d/vc-projects?
>
> Not necessarily duplicated, because no one said the projects I use
> will necessarily visit all (indeed, even one of) the VC roots that I
> will visit via VC commands.  And vice versa.
>
> IOW, please don't assume that, because project.el can use the VC
> backend, it will always use VC in the same directories.  If the roots
> are different, offering the same completion candidates will be
> confusing and even annoying.

OTOH, it would be confusing and annoying to have duplicate histories for
users who prefer project.el solely for handling vc repositories.

>> > Moreover, using a command such as "C-x v d" doesn't mean I want
>> > project.el record that, again because I might be using project.el
>> > commands for projects that aren't based on VC.
>>
>> Then project.el could have a customizable variable that will prevent
>> recording vc project history.
>
> What for? let every layer record its own history, and then use the
> relevant history in each layer.
>
>> > And if the history is collected by VC, it could be made available to
>> > project.el commands that call into VC, right?
>>
>> You mean to record an internal vc-history separately from project.el,
>> but still to save it in the same project file ~/.emacs.d/projects?
>
> No, I mean provide a possibility for project.el to access the history
> saved in ~/.emacs.d/vc-history, in addition to project.el's own
> history.  If the user wants that.

This is what I meant by a customizable variable mentioned above
to toggle recording of a separate vc history.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 29.06.2020 00:51, Juri Linkov wrote:

> So analogously, vc.el is one of back-ends of project.el, so in vc.el
> there should be such line for using project functions:
> 
>    (require 'project)

Not really.

project-vc (which is the project backend) resides in project.el.

It calls to some VC functions, but VC itself needs no awareness of 
project.el.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 29.06.2020 00:49, Juri Linkov wrote:
>>>> both the name and the docstring need to be changed
>>> I can't believe that you renamed project-known-roots to
>>> project-known-project-roots.  Why duplicate prefixes 'project-'?
>>
>> To emphasize that the roots belong to different projects, and not the
>> current one.
> 
> Oh, I see that project can have several roots with project-roots and
> project-external-roots.

project-roots is now obsolete, but the external roots are still here.

> But still would it be possible to deuglify the
> name project-known-project-roots?  Maybe project-all-known-roots?

Is it so really so ugly? Note that we already have 
project-prompt-project-dir and project-switch-project (an interactive 
command).

project-all-known-roots is better, but still quite reminiscent of an 
all-known-roots slots on a "project struct". You wouldn't mistake 
project-known-project-roots for such.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Juri Linkov]

>> So analogously, vc.el is one of back-ends of project.el, so in vc.el
>> there should be such line for using project functions:
>>    (require 'project)
>
> Not really.
>
> project-vc (which is the project backend) resides in project.el.
>
> It calls to some VC functions, but VC itself needs no awareness of
> project.el.

I see that project.el calls some functions from preloaded vc-hooks.el
and some autoloaded functions from vc.el.

According to this design, this feature should be implemented the same way.
In 'vc-root-diff' command:

  (read-directory-name
   "Directory for VC root-diff: " nil
   (when (featurep 'project)
     (project-vc-known-roots)))

Note the same prefix 'project-vc' for vc backend of project.el.
Then 'project-vc-known-roots' in project.el be like this:

(defun project-vc-known-roots ()
  "Return a list of known vc roots."
  (seq-filter #'project-try-vc (project-known-project-roots)))

Also every command that visits a directory in vc could register their dir
in the project list when project.el is loaded, for example:

  (defun vc-dir (dir &optional backend)
   ...
   (when (featurep 'project)
    ;; Add current vc project dir to project list
    (let ((default-directory dir))
     (project-current t)))

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 30.06.2020 01:58, Juri Linkov wrote:

> Note the same prefix 'project-vc' for vc backend of project.el.
> Then 'project-vc-known-roots' in project.el be like this:
> 
> (defun project-vc-known-roots ()
>    "Return a list of known vc roots."
>    (seq-filter #'project-try-vc (project-known-project-roots)))

You suggested this before. Special-casing a particular backend in a 
general purpose public function is not a good idea.

> Also every command that visits a directory in vc could register their dir
> in the project list when project.el is loaded, for example:
> 
>    (defun vc-dir (dir &optional backend)
>     ...
>     (when (featurep 'project)
>      ;; Add current vc project dir to project list
>      (let ((default-directory dir))
>       (project-current t)))

Having considered it more, I now have more doubt on whether this 
approach is a good idea in general.

See, even when the project backend is VC, there is no guarantee that its 
root will be the repository's root. First, there are submodules (and 
whether a submodule root is a project root is customizable). Second, 
there is an existing feature request to use also some other project root 
markers, even inside VC repos (the "monorepo" case). These might end up 
in the 'vc' backend as well.

So things as they are, I'd rather VC has a separate roots history, or we 
at least put this feature request on hold (and, for now, revert the 
installed patches).

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Juri Linkov]

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

>> Note the same prefix 'project-vc' for vc backend of project.el.
>> Then 'project-vc-known-roots' in project.el be like this:
>> (defun project-vc-known-roots ()
>>    "Return a list of known vc roots."
>>    (seq-filter #'project-try-vc (project-known-project-roots)))
>
> You suggested this before. Special-casing a particular backend in a general
> purpose public function is not a good idea.

It's not a general purpose function.  Its prefix 'project-vc-' indicates that
this public function is specific to project-vc backend.

>> Also every command that visits a directory in vc could register their dir
>> in the project list when project.el is loaded, for example:
>>    (defun vc-dir (dir &optional backend)
>>     ...
>>     (when (featurep 'project)
>>      ;; Add current vc project dir to project list
>>      (let ((default-directory dir))
>>       (project-current t)))
>
> Having considered it more, I now have more doubt on whether this approach
> is a good idea in general.
>
> See, even when the project backend is VC, there is no guarantee that its
> root will be the repository's root.

Isn't the project's root the same as the repository's root in 99% cases?

> First, there are submodules (and whether a submodule root is a project
> root is customizable).

This is a general problem currently discussed in other threads.

> Second, there is an existing feature request to use also some other
> project root markers, even inside VC repos (the "monorepo"
> case). These might end up in the 'vc' backend as well.

I don't see how is this related to the subject of this bug report.
It's not a big problem when some directories provided by M-n
are not repository root directories in 1% of cases.

> So things as they are, I'd rather VC has a separate roots history,
> or we at least put this feature request on hold (and, for now, revert
> the installed patches).

Look, what I'm trying to say is that there are users who want
to use project directories from ~/.emacs.d/projects in vc commands.
Of course, there are users with no free memory available that can't
afford loading project.el to the memory.  Addressing the needs of users
who don't use project.el is a separate issue.  But in this report
I'm addressing the needs of users who want to use project.el in vc commands.

Also I'm sure that most users will want to use project directories from
~/.emacs.d/projects not only in vc commands, but in more places for
non-vc commands.  Here's a similar patch for grep commands:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: read-directory-name-project-known-project-roots.patch --]
[-- Type: text/x-diff, Size: 2102 bytes --]

diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index 7731be5965..331abf0f38 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -1078,8 +1078,12 @@ lgrep
        (error "grep.el: No `grep-template' available"))
       (t (let* ((regexp (grep-read-regexp))
 		(files (grep-read-files regexp))
-		(dir (read-directory-name "In directory: "
-					  nil default-directory t))
+		(dir (read-directory-name
+                      "In directory: " nil
+                      (if (featurep 'project)
+                          (project-known-project-roots)
+                        default-directory)
+                      t))
 		(confirm (equal current-prefix-arg '(4))))
 	   (list regexp files dir confirm))))))
   (when (and (stringp regexp) (> (length regexp) 0))
@@ -1166,8 +1170,12 @@ rgrep
        (error "grep.el: No `grep-find-template' available"))
       (t (let* ((regexp (grep-read-regexp))
 		(files (grep-read-files regexp))
-		(dir (read-directory-name "Base directory: "
-					  nil default-directory t))
+		(dir (read-directory-name
+                      "Base directory: " nil
+                      (if (featurep 'project)
+                          (project-known-project-roots)
+                        default-directory)
+                      t))
 		(confirm (equal current-prefix-arg '(4))))
 	   (list regexp files dir confirm))))))
   (when (and (stringp regexp) (> (length regexp) 0))
@@ -1297,8 +1305,12 @@ zrgrep
 	 (error "grep.el: No `grep-find-template' available"))
 	(t (let* ((regexp (grep-read-regexp))
 		  (files (grep-read-files regexp))
-		  (dir (read-directory-name "Base directory: "
-					    nil default-directory t))
+		  (dir (read-directory-name
+                        "Base directory: " nil
+                        (if (featurep 'project)
+                            (project-known-project-roots)
+                          default-directory)
+                        t))
 		  (confirm (equal current-prefix-arg '(4))))
 	     (list regexp files dir confirm grep-find-template)))))))
   (let ((grep-find-template template)

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 30.06.2020 23:50, Juri Linkov wrote:
>>> Note the same prefix 'project-vc' for vc backend of project.el.
>>> Then 'project-vc-known-roots' in project.el be like this:
>>> (defun project-vc-known-roots ()
>>>     "Return a list of known vc roots."
>>>     (seq-filter #'project-try-vc (project-known-project-roots)))
>>
>> You suggested this before. Special-casing a particular backend in a general
>> purpose public function is not a good idea.
> 
> It's not a general purpose function.  Its prefix 'project-vc-' indicates that
> this public function is specific to project-vc backend.

An yet, its purpose is general. If it were to be used ubiquitously, it 
should remain useful even if the user employs other backends, not 
project-vc.

>> See, even when the project backend is VC, there is no guarantee that its
>> root will be the repository's root.
> 
> Isn't the project's root the same as the repository's root in 99% cases?

This number is going to get lower in the future. Because of the examples 
I cited, and maybe others.

>> First, there are submodules (and whether a submodule root is a project
>> root is customizable).
> 
> This is a general problem currently discussed in other threads.

I don't understand this response.

>> Second, there is an existing feature request to use also some other
>> project root markers, even inside VC repos (the "monorepo"
>> case). These might end up in the 'vc' backend as well.
> 
> I don't see how is this related to the subject of this bug report.
> It's not a big problem when some directories provided by M-n
> are not repository root directories in 1% of cases.

It is related to the sentence I wrote above it in the previous email. 
And it's related to your estimation of "99%".

>> So things as they are, I'd rather VC has a separate roots history,
>> or we at least put this feature request on hold (and, for now, revert
>> the installed patches).
> 
> Look, what I'm trying to say is that there are users who want
> to use project directories from ~/.emacs.d/projects in vc commands.
> Of course, there are users with no free memory available that can't
> afford loading project.el to the memory.  Addressing the needs of users
> who don't use project.el is a separate issue.  But in this report
> I'm addressing the needs of users who want to use project.el in vc commands.

The question is whether it would be better done in a different way.

And consider: okay, saving a separate (partially duplicating) list in 
~/.emacs.d/vc-roots sounds somewhat of a hassle, but from the user's 
POV, they will enter a repository root directory one more time. And from 
then on Emacs will suggest it from the saved list, and all will be well. 
  It could even be memorized automatically most of the time, similarly 
to what you suggested in the previous email.

And in the cases when a project root doesn't match the repository root, 
this alternative solution will result in better behavior.

> Also I'm sure that most users will want to use project directories from
> ~/.emacs.d/projects not only in vc commands, but in more places for
> non-vc commands.  Here's a similar patch for grep commands:

This looks like a substitute for a 'project-grep' command, right?

But if Eli thinks it's good, I have no objection. Note that the 
tradeoffs are different in this case: obviously, there's even no need to 
filter by #'project-try-vc.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 1 Jul 2020 00:59:59 +0300
> Cc: 41821@debbugs.gnu.org
> 
> > Look, what I'm trying to say is that there are users who want
> > to use project directories from ~/.emacs.d/projects in vc commands.
> > Of course, there are users with no free memory available that can't
> > afford loading project.el to the memory.  Addressing the needs of users
> > who don't use project.el is a separate issue.  But in this report
> > I'm addressing the needs of users who want to use project.el in vc commands.
> 
> The question is whether it would be better done in a different way.
> 
> And consider: okay, saving a separate (partially duplicating) list in 
> ~/.emacs.d/vc-roots sounds somewhat of a hassle, but from the user's 
> POV, they will enter a repository root directory one more time. And from 
> then on Emacs will suggest it from the saved list, and all will be well. 
>   It could even be memorized automatically most of the time, similarly 
> to what you suggested in the previous email.
> 
> And in the cases when a project root doesn't match the repository root, 
> this alternative solution will result in better behavior.
> 
> > Also I'm sure that most users will want to use project directories from
> > ~/.emacs.d/projects not only in vc commands, but in more places for
> > non-vc commands.  Here's a similar patch for grep commands:
> 
> This looks like a substitute for a 'project-grep' command, right?
> 
> But if Eli thinks it's good, I have no objection.

Like you, Dmitry, I'm a bit uneasy with mixing the two sets of
features.  We should decide on some concept and try to stick to it;
right now, it seems to me that we prefer to have specialized commands
in project.el rather than inject project.el-specific nits into
commands outside project.el, which I think could be a slippery slope.
Why isn't that a better approach?  I don't think it's wise to blur the
difference between using project.el features and the VC back-end
features that support them.  If someone wants to use project.el in VC
commands, let them use project.el commands, not VC commands.  That
way, Emacs will know that some kind of project is being worked on, and
could offer more targeted support for such users.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

Hi Eli,

On 01.07.2020 17:42, Eli Zaretskii wrote:
> Like you, Dmitry, I'm a bit uneasy with mixing the two sets of
> features.  We should decide on some concept and try to stick to it;
> right now, it seems to me that we prefer to have specialized commands
> in project.el rather than inject project.el-specific nits into
> commands outside project.el, which I think could be a slippery slope.

In my previous message I drew a line, of sorts, where I think it's okay 
to suggest directories to search in in rgrep from known project roots. I 
do think it's okay.

I'm not sure if I understand your position, because you say you agree, 
but then make a one-directional statement.

> Why isn't that a better approach?  I don't think it's wise to blur the
> difference between using project.el features and the VC back-end
> features that support them.  If someone wants to use project.el in VC
> commands, let them use project.el commands, not VC commands.  That
> way, Emacs will know that some kind of project is being worked on, and
> could offer more targeted support for such users.

Not sure that's going to result in optimal user experience. After all, 
simply having a copy of every command, but acting on a project, would 
make it 2x the number of commands.

And project-rgrep, on the other hand, would probably search the current 
project root without prompting. Unlike the proposed change to rgrep, 
which only makes it a suggestion.

Another long-term violation of your idea is the default definition of 
xref-backend-references. It uses the current project. You could say that 
mixes up abstractions as well, but it's just too handy to implement this 
way.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Juri Linkov]

> we at least put this feature request on hold (and, for now,
> revert the installed patches).

Ok, reverted the installed patches.

It's easy to achieve the same in the init file with these
simple advises.  Now this works perfectly from POV of users
who prefer to use project directories everywhere in all prompts
that ask for a directory:

#+begin_src emacs-lisp
;; When a prompt of some commands such as 'rgrep' or 'vc-print-log' asks for
;; a directory name, allow 'M-n' to access the most recently used project
;; directories saved in ~/.emacs.d/projects:

(advice-add 'read-directory-name :around
            (lambda (orig-fun prompt &optional dir default-dirname
                              mustmatch initial)
              (when (featurep 'project)
                (setq default-dirname
                      (append (cond
                               ((null default-dirname)
                                (list (or dir default-directory)))
                               ((consp default-dirname)
                                default-dirname)
                               (t
                                (list default-dirname)))
                              (project-known-project-roots))))
              (let ((ret (funcall orig-fun prompt dir default-dirname
                                  mustmatch initial)))
                (when (featurep 'project)
                  ;; Update project list with selected project dir
                  (let ((default-directory ret))
                    (project-current t)))
                ret))
            '((name . read-directory-name-project-defaults)))

(advice-add 'vc-dir :after
            (lambda (dir &optional _backend)
              (when (featurep 'project)
                ;; Add current vc project dir to project list
                (let ((default-directory dir))
                  (project-current t))))
            '((name . vc-dir-add-project)))
#+end_src

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Eli Zaretskii]

> Cc: 41821@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 1 Jul 2020 23:24:19 +0300
> 
> On 01.07.2020 17:42, Eli Zaretskii wrote:
> > Like you, Dmitry, I'm a bit uneasy with mixing the two sets of
> > features.  We should decide on some concept and try to stick to it;
> > right now, it seems to me that we prefer to have specialized commands
> > in project.el rather than inject project.el-specific nits into
> > commands outside project.el, which I think could be a slippery slope.
> 
> In my previous message I drew a line, of sorts, where I think it's okay 
> to suggest directories to search in in rgrep from known project roots. I 
> do think it's okay.
> 
> I'm not sure if I understand your position, because you say you agree, 
> but then make a one-directional statement.

I agreed with your general doubt whether the proposal is TRT.  My
motivation is somewhat different.

> > Why isn't that a better approach?  I don't think it's wise to blur the
> > difference between using project.el features and the VC back-end
> > features that support them.  If someone wants to use project.el in VC
> > commands, let them use project.el commands, not VC commands.  That
> > way, Emacs will know that some kind of project is being worked on, and
> > could offer more targeted support for such users.
> 
> Not sure that's going to result in optimal user experience. After all, 
> simply having a copy of every command, but acting on a project, would 
> make it 2x the number of commands.
> 
> And project-rgrep, on the other hand, would probably search the current 
> project root without prompting. Unlike the proposed change to rgrep, 
> which only makes it a suggestion.

I just fear that this is a slippery slope: we will eventually need to
inject this into many GP commands, "for consistency".

> Another long-term violation of your idea is the default definition of 
> xref-backend-references. It uses the current project. You could say that 
> mixes up abstractions as well, but it's just too handy to implement this 
> way.

I don't think I understand the issue and the use case, sorry.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 02.07.2020 01:10, Juri Linkov wrote:
>> we at least put this feature request on hold (and, for now,
>> revert the installed patches).
> 
> Ok, reverted the installed patches.
> 
> It's easy to achieve the same in the init file with these
> simple advises.  Now this works perfectly from POV of users
> who prefer to use project directories everywhere in all prompts
> that ask for a directory:
> 
> #+begin_src emacs-lisp
> ;; When a prompt of some commands such as 'rgrep' or 'vc-print-log' asks for
> ;; a directory name, allow 'M-n' to access the most recently used project
> ;; directories saved in ~/.emacs.d/projects:
> 
> (advice-add 'read-directory-name :around

I figured you might want to define a vc-specific wrapper function for 
read-directory-name, for ease of advising. But now I'm not so sure.

>                  (when (featurep 'project)
>                    ;; Update project list with selected project dir
>                    (let ((default-directory ret))
>                      (project-current t)))

The 't' argument is counter-productive here. You don't want it to 
prompt, which will happen when the target directory is in the end 
outside of any detected projects.

And really if you just do this in an ad-hoc advice (and recommend it to 
other users), it's probably better to just remove this call for 
simplicity. After all, you probably interact with "proper" project 
commands anyway. And they will put the current project at the top of the 
history.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 02.07.2020 16:36, Eli Zaretskii wrote:

>>> Why isn't that a better approach?  I don't think it's wise to blur the
>>> difference between using project.el features and the VC back-end
>>> features that support them.  If someone wants to use project.el in VC
>>> commands, let them use project.el commands, not VC commands.  That
>>> way, Emacs will know that some kind of project is being worked on, and
>>> could offer more targeted support for such users.
>>
>> Not sure that's going to result in optimal user experience. After all,
>> simply having a copy of every command, but acting on a project, would
>> make it 2x the number of commands.
>>
>> And project-rgrep, on the other hand, would probably search the current
>> project root without prompting. Unlike the proposed change to rgrep,
>> which only makes it a suggestion.
> 
> I just fear that this is a slippery slope: we will eventually need to
> inject this into many GP commands, "for consistency".

The notion of current project is very common in most editors, so I 
imagine this will be TRT, sooner or later. Not that I'm in a hurry for that.

>> Another long-term violation of your idea is the default definition of
>> xref-backend-references. It uses the current project. You could say that
>> mixes up abstractions as well, but it's just too handy to implement this
>> way.
> 
> I don't think I understand the issue and the use case, sorry.

The use case is 'M-x xref-find-refereces' and xref backends which don't 
override xref-backend-references. In which case this command searches 
the current project using general purpose tools (one of semantic symref 
tools, or Grep).

But xref backend != current project. They're technically and 
theoretically independent.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Eli Zaretskii]

> Cc: 41821@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 2 Jul 2020 17:43:29 +0300
> 
> > I just fear that this is a slippery slope: we will eventually need to
> > inject this into many GP commands, "for consistency".
> 
> The notion of current project is very common in most editors

But Emacs is more than just an IDE, it can be and is used for many
other jobs.  For example, I customary take a break from my development
work to read email, and when doing so I might issue some Grep command
that I need for some email message I'm writing.  I don't think it's
right for Emacs to assume that every Grep I do is necessarily related
to the last project I was working on (which could be days in the past,
btw).  This way, we would need a command to "get out of" (or "close")
the project, which I think would be both a nuisance and absurd.

> >> Another long-term violation of your idea is the default definition of
> >> xref-backend-references. It uses the current project. You could say that
> >> mixes up abstractions as well, but it's just too handy to implement this
> >> way.
> > 
> > I don't think I understand the issue and the use case, sorry.
> 
> The use case is 'M-x xref-find-refereces' and xref backends which don't 
> override xref-backend-references. In which case this command searches 
> the current project using general purpose tools (one of semantic symref 
> tools, or Grep).
> 
> But xref backend != current project. They're technically and 
> theoretically independent.

So you are saying that it might bring me the wrong references once in
a while?  That's not good, is it?

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 02.07.2020 20:41, Eli Zaretskii wrote:

>> The notion of current project is very common in most editors
> 
> But Emacs is more than just an IDE, it can be and is used for many
> other jobs.  For example, I customary take a break from my development
> work to read email, and when doing so I might issue some Grep command
> that I need for some email message I'm writing.  I don't think it's
> right for Emacs to assume that every Grep I do is necessarily related
> to the last project I was working on (which could be days in the past,
> btw).  This way, we would need a command to "get out of" (or "close")
> the project, which I think would be both a nuisance and absurd.

But it's for a default value (one you can insert using M-n, or not). 
Most users won't even notice this.

>> The use case is 'M-x xref-find-refereces' and xref backends which don't
>> override xref-backend-references. In which case this command searches
>> the current project using general purpose tools (one of semantic symref
>> tools, or Grep).
>>
>> But xref backend != current project. They're technically and
>> theoretically independent.
> 
> So you are saying that it might bring me the wrong references once in
> a while?  That's not good, is it?

If an xref backend doesn't define the xref-backend-references method, 
the alternative is no references at all.

Anything else we do, will be imprecise. But it's useful to have a 
default with "good enough" behavior, as you have probably noticed.

Whether the current implementation will give wrong results, and how 
often, is difficult for me to predict. It also depends on what we 
consider a "wrong reference". etags and elisp backends don't always give 
perfect results for "find definition" either.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Eli Zaretskii]

> Cc: 41821@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 2 Jul 2020 22:37:36 +0300
> 
> > But Emacs is more than just an IDE, it can be and is used for many
> > other jobs.  For example, I customary take a break from my development
> > work to read email, and when doing so I might issue some Grep command
> > that I need for some email message I'm writing.  I don't think it's
> > right for Emacs to assume that every Grep I do is necessarily related
> > to the last project I was working on (which could be days in the past,
> > btw).  This way, we would need a command to "get out of" (or "close")
> > the project, which I think would be both a nuisance and absurd.
> 
> But it's for a default value (one you can insert using M-n, or not). 
> Most users won't even notice this.

You assume that most users don't know about or use M-n?  I do it all
the time, and would like to think others do as well.

> >> The use case is 'M-x xref-find-refereces' and xref backends which don't
> >> override xref-backend-references. In which case this command searches
> >> the current project using general purpose tools (one of semantic symref
> >> tools, or Grep).
> >>
> >> But xref backend != current project. They're technically and
> >> theoretically independent.
> > 
> > So you are saying that it might bring me the wrong references once in
> > a while?  That's not good, is it?
> 
> If an xref backend doesn't define the xref-backend-references method, 
> the alternative is no references at all.

The alternative could be to start with the current directory, or ask
the user.

But do we have xref backends that don't define the
xref-backend-references method?  If so, which ones don't?

> Whether the current implementation will give wrong results, and how 
> often, is difficult for me to predict. It also depends on what we 
> consider a "wrong reference". etags and elisp backends don't always give 
> perfect results for "find definition" either.

"Imperfect" and "completely wrong" is not the same at all.  Searching
the wrong directory hierarchy will get you the latter.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 03.07.2020 08:55, Eli Zaretskii wrote:

>> But it's for a default value (one you can insert using M-n, or not).
>> Most users won't even notice this.
> 
> You assume that most users don't know about or use M-n?  I do it all
> the time, and would like to think others do as well.

I assume, yes.

But even if they do, I'm not sure how this extra capability could harm them.

>>>> The use case is 'M-x xref-find-refereces' and xref backends which don't
>>>> override xref-backend-references. In which case this command searches
>>>> the current project using general purpose tools (one of semantic symref
>>>> tools, or Grep).
>>>>
>>>> But xref backend != current project. They're technically and
>>>> theoretically independent.
>>>
>>> So you are saying that it might bring me the wrong references once in
>>> a while?  That's not good, is it?
>>
>> If an xref backend doesn't define the xref-backend-references method,
>> the alternative is no references at all.
> 
> The alternative could be to start with the current directory, or ask
> the user.

Using the current directory will result in worse results almost always, 
IME. Prompting the user every time is not a great experience either.

> But do we have xref backends that don't define the
> xref-backend-references method?  If so, which ones don't?

Neither of the built-in ones does. :-)

>> Whether the current implementation will give wrong results, and how
>> often, is difficult for me to predict. It also depends on what we
>> consider a "wrong reference". etags and elisp backends don't always give
>> perfect results for "find definition" either.
> 
> "Imperfect" and "completely wrong" is not the same at all.  Searching
> the wrong directory hierarchy will get you the latter.

Perhaps. To get better results, the user will have to set up the project 
configuration better.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Eli Zaretskii]

> Cc: 41821@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 3 Jul 2020 13:23:05 +0300
> 
> > You assume that most users don't know about or use M-n?  I do it all
> > the time, and would like to think others do as well.
> 
> I assume, yes.
> 
> But even if they do, I'm not sure how this extra capability could harm them.

??? Offering incorrect default doesn't cause harm?

> >> If an xref backend doesn't define the xref-backend-references method,
> >> the alternative is no references at all.
> > 
> > The alternative could be to start with the current directory, or ask
> > the user.
> 
> Using the current directory will result in worse results almost always, 

Worse than searching unrelated directories?  Perhaps you are thinking
about the use case where the project-related directories are indeed
relevant to the search.  I was thinking about the opposite use case.

> IME. Prompting the user every time is not a great experience either.

Not every time, only the first time.  Afterwards, the user has the
history.

> > But do we have xref backends that don't define the
> > xref-backend-references method?  If so, which ones don't?
> 
> Neither of the built-in ones does. :-)

But currently, they don't offer project-related defaults, do they?  At
least not in released versions of Emacs.

> >> Whether the current implementation will give wrong results, and how
> >> often, is difficult for me to predict. It also depends on what we
> >> consider a "wrong reference". etags and elisp backends don't always give
> >> perfect results for "find definition" either.
> > 
> > "Imperfect" and "completely wrong" is not the same at all.  Searching
> > the wrong directory hierarchy will get you the latter.
> 
> Perhaps. To get better results, the user will have to set up the project 
> configuration better.

Like what?  I don't think I understand how project configuration could
help in non project-related searches.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Dmitry Gutov]

On 03.07.2020 14:48, Eli Zaretskii wrote:
>> Cc: 41821@debbugs.gnu.org, juri@linkov.net
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Fri, 3 Jul 2020 13:23:05 +0300
>>
>>> You assume that most users don't know about or use M-n?  I do it all
>>> the time, and would like to think others do as well.
>>
>> I assume, yes.
>>
>> But even if they do, I'm not sure how this extra capability could harm them.
> 
> ??? Offering incorrect default doesn't cause harm?

Offering known project roots directories would be incorrect?

>>>> If an xref backend doesn't define the xref-backend-references method,
>>>> the alternative is no references at all.
>>>
>>> The alternative could be to start with the current directory, or ask
>>> the user.
>>
>> Using the current directory will result in worse results almost always,
> 
> Worse than searching unrelated directories?

Worse than the current behavior.

And either way, using just the current directory will almost always 
result in *missing* valid references from the results. That's much worse 
than including some irrelevant results, potentially.

> Perhaps you are thinking
> about the use case where the project-related directories are indeed
> relevant to the search.  I was thinking about the opposite use case.

I'm not sure about the totality of the use cases that would be helped or 
harmed by the current approach. But I have yet to see a single user 
complain about the current behavior.

>> IME. Prompting the user every time is not a great experience either.
> 
> Not every time, only the first time.  Afterwards, the user has the
> history.

They can only input one directory this way. A project can include 
"external roots".

>>> But do we have xref backends that don't define the
>>> xref-backend-references method?  If so, which ones don't?
>>
>> Neither of the built-in ones does. :-)
> 
> But currently, they don't offer project-related defaults, do they?  At
> least not in released versions of Emacs.

What do you mean? Doesn't 'M-x xref-find-references' work for you?

This situation has been in place for ~5 years now.

>>> "Imperfect" and "completely wrong" is not the same at all.  Searching
>>> the wrong directory hierarchy will get you the latter.
>>
>> Perhaps. To get better results, the user will have to set up the project
>> configuration better.
> 
> Like what?

Like using a project backend that would describe the locations of their 
source files better.

> I don't think I understand how project configuration could
> help in non project-related searches.

And you lost me here. xref backend doesn't equal a project backend, but 
you can't claim they are entirely unrelated either (from the user's 
point of view), or that their configurations don't correlate heavily.

bug#41821: 28.0.50; read-directory-name in vc commands should provide defaults from projects

[Drew Adams]

> > But it's for a default value (one you can insert using M-n, or not).
> > Most users won't even notice this.
> 
> You assume that most users don't know about or use M-n?  I do it all
> the time, and would like to think others do as well.

OT -

Dunno about "most", but I suspect "many", yes.

`M-n' is super-useful, and I'm guessing that many
users, ignorant of it, fairly often go to the
trouble of typing what's in fact the default value.

Dunno what might be a good solution to this lack
of discovery.

I've taken one approach that I hope helps a bit,
but it too suffers from discoverability.  Pretty
much anything that doesn't, by default, put the
default value in the minibuffer suffers from some
difficulty of discovery.

What I do in Icicles is this, FWIW (again, not a
panacea):

When option `icicle-default-value' is non-nil (it's
nil by default, hence a discovery problem still),
and the INITIAL-INPUT arg of minibuffer-reading
functions is nil or "", the DEFAULT-VALUE arg can
be inserted in the minibuffer as input.

(You can clear the minibuffer anytime with `M-k'.)

Particular non-nil values determine placement of
point wrt the inserted value, and whether the value
is automatically selected (useful with
`delete-selection-mode').

https://www.emacswiki.org/emacs/Icicles_-_Customization_and_General_Tips#icicle-default-value

I mention this because I think that `M-n' is likely
underused because of ignorance.  Other ways to make
it known could help.  Inserting it automatically is
not necessarily the best way.

We could, say, until you do something to remove the
nag, append something to minibuffer prompts, to
point out `M-n'.  Or we could provide a help key
that shows *Help* with info about `M-n', `M-p', etc.
Dunno what might help new users the most without
bothering others.