GNU bug report logs - #39502
[PATCH] Use one src status -a call for vc-src-dir-status-files

Previous Next

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

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

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

From: Wolfgang Scherer <Wolfgang.Scherer <at> gmx.de>
To: Emacs Bugs <bug-gnu-emacs <at> gnu.org>
Subject: [PATCH] Use one src status -a call for vc-src-dir-status-files
Date: Fri, 7 Feb 2020 23:52:45 +0100

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

While working on the ignore feature for SRC, I found that unregistered files were not displayed in vc-dir-mode.

The source code specified in a comment:

   ;; FIXME: Use one src status -a call for this

Here is a patch that implements that.

[0001-Use-one-src-status-a-call-for-vc-src-dir-status-file.patch (text/x-patch, attachment)]

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

From: Wolfgang Scherer <Wolfgang.Scherer <at> gmx.de>
To: 39502 <at> debbugs.gnu.org
Subject: Re: bug#39502: Acknowledgement ([PATCH] Use one src status -a call
 for vc-src-dir-status-files)
Date: Sat, 8 Feb 2020 06:35:01 +0100

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

Additional recursion into sub-directories.

[0001-Use-one-src-status-a-call-for-vc-src-dir-status-file.patch (text/x-patch, attachment)]

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Wolfgang Scherer <Wolfgang.Scherer <at> gmx.de>, 39502 <at> debbugs.gnu.org
Subject: Re: bug#39502: [PATCH] Use one src status -a call for
 vc-src-dir-status-files
Date: Sat, 8 Feb 2020 12:39:04 +0300

Hi Wolfgang,

On 08.02.2020 1:52, Wolfgang Scherer wrote:
> Here is a patch that implements that.

Hopefully someone familiar with SRC can review this. If that doesn't 
happen in a reasonable amount of time, you (or someone else) can please 
go ahead and install the latest patch version on master.

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

From: Wolfgang Scherer <Wolfgang.Scherer <at> gmx.de>
To: Dmitry Gutov <dgutov <at> yandex.ru>, 39502 <at> debbugs.gnu.org
Subject: Re: bug#39502: [PATCH] Use one src status -a call for
 vc-src-dir-status-files
Date: Mon, 10 Feb 2020 01:47:54 +0100

Hi Dmitry,

Am 08.02.20 um 10:39 schrieb Dmitry Gutov:
>
> On 08.02.2020 1:52, Wolfgang Scherer wrote:
>> Here is a patch that implements that.
>
> Hopefully someone familiar with SRC can review this. If that doesn't happen in a reasonable amount of time, you (or someone else) can please go ahead and install the latest patch version on master.
I am not sure what you mean by "install the latest patch version on master"?

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Wolfgang Scherer <Wolfgang.Scherer <at> gmx.de>, 39502 <at> debbugs.gnu.org
Subject: Re: bug#39502: [PATCH] Use one src status -a call for
 vc-src-dir-status-files
Date: Mon, 10 Feb 2020 07:29:31 +0300

On 10.02.2020 3:47, Wolfgang Scherer wrote:
> Hi Dmitry,
> 
> Am 08.02.20 um 10:39 schrieb Dmitry Gutov:
>>
>> On 08.02.2020 1:52, Wolfgang Scherer wrote:
>>> Here is a patch that implements that.
>>
>> Hopefully someone familiar with SRC can review this. If that doesn't happen in a reasonable amount of time, you (or someone else) can please go ahead and install the latest patch version on master.
> I am not sure what you mean by "install the latest patch version on master"?

You posted two versions. Maybe there will be further revisions.

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

From: Wolfgang Scherer <Wolfgang.Scherer <at> gmx.de>
To: Dmitry Gutov <dgutov <at> yandex.ru>, 39502 <at> debbugs.gnu.org
Subject: Re: bug#39502: [PATCH] Use one src status -a call for
 vc-src-dir-status-files
Date: Tue, 11 Feb 2020 03:54:03 +0100

Am 10.02.20 um 05:29 schrieb Dmitry Gutov:
> On 10.02.2020 3:47, Wolfgang Scherer wrote:
>> Hi Dmitry,
>>
>> Am 08.02.20 um 10:39 schrieb Dmitry Gutov:
>>>
>>> On 08.02.2020 1:52, Wolfgang Scherer wrote:
>>>> Here is a patch that implements that.
>>>
>>> Hopefully someone familiar with SRC can review this. If that doesn't happen in a reasonable amount of time, you (or someone else) can please go ahead and install the latest patch version on master.
>> I am not sure what you mean by "install the latest patch version on master"?
>
> You posted two versions. Maybe there will be further revisions.
I am not planning on any :-). But I am still not sure, which master you mean. I have no write access to the repository.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Wolfgang Scherer <Wolfgang.Scherer <at> gmx.de>, 39502 <at> debbugs.gnu.org
Subject: Re: bug#39502: [PATCH] Use one src status -a call for
 vc-src-dir-status-files
Date: Tue, 11 Feb 2020 09:46:10 +0200

On 11.02.2020 4:54, Wolfgang Scherer wrote:
> 
> Am 10.02.20 um 05:29 schrieb Dmitry Gutov:
>> On 10.02.2020 3:47, Wolfgang Scherer wrote:
>>> Hi Dmitry,
>>>
>>> Am 08.02.20 um 10:39 schrieb Dmitry Gutov:
>>>>
>>>> On 08.02.2020 1:52, Wolfgang Scherer wrote:
>>>>> Here is a patch that implements that.
>>>>
>>>> Hopefully someone familiar with SRC can review this. If that doesn't happen in a reasonable amount of time, you (or someone else) can please go ahead and install the latest patch version on master.
>>> I am not sure what you mean by "install the latest patch version on master"?
>>
>> You posted two versions. Maybe there will be further revisions.
> I am not planning on any :-). But I am still not sure, which master you mean. I have no write access to the repository.

The Emacs master branch. That's why I said "or someone else".

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Wolfgang Scherer <Wolfgang.Scherer <at> gmx.de>
Cc: 39502 <at> debbugs.gnu.org, dgutov <at> yandex.ru
Subject: Re: bug#39502: [PATCH] Use one src status -a call for
 vc-src-dir-status-files
Date: Fri, 14 Feb 2020 12:05:47 +0200

> From: Wolfgang Scherer <Wolfgang.Scherer <at> gmx.de>
> Date: Tue, 11 Feb 2020 03:54:03 +0100
> 
> >> I am not sure what you mean by "install the latest patch version on master"?
> >
> > You posted two versions. Maybe there will be further revisions.
> I am not planning on any :-). But I am still not sure, which master you mean. I have no write access to the repository.

Can you describe a simple use case to test your patch, and tell what
are the differences between the situation before and after the patch?
Then I will try the changes, and install them if I see no problems.

(I'm asking for details because at least part of the changes are
simply white-space changes, and it isn't easy to identify the part(s)
that really change the code workings.)

Thanks.

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

From: Wolfgang Scherer <Wolfgang.Scherer <at> gmx.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 39502 <at> debbugs.gnu.org, dgutov <at> yandex.ru
Subject: Re: bug#39502: [PATCH] Use one src status -a call for
 vc-src-dir-status-files
Date: Sat, 15 Feb 2020 03:44:40 +0100

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

Am 14.02.20 um 11:05 schrieb Eli Zaretskii:
>> From: Wolfgang Scherer <Wolfgang.Scherer <at> gmx.de>
>> Date: Tue, 11 Feb 2020 03:54:03 +0100
>>
>>>> I am not sure what you mean by "install the latest patch version on master"?
>>> You posted two versions. Maybe there will be further revisions.
>> I am not planning on any :-). But I am still not sure, which master you mean. I have no write access to the repository.
> Can you describe a simple use case to test your patch, and tell what
> are the differences between the situation before and after the patch?
> Then I will try the changes, and install them if I see no problems.
>
> (I'm asking for details because at least part of the changes are
> simply white-space changes, and it isn't easy to identify the part(s)
> that really change the code workings.)
Sorry, I have a whitespace cleaner command that I keep using
unconciously.

The use case is to have a SRC repository with another SRC sub-repositoy,
which in turn contains a further SRC repository. When entering
*vc-dir-mode* with `M-x vc-dir` in the topmost repository, the status of
all files in all 3 respositories should be shown.

OK, the change is not so hard to understand when you know the sequence
of events.

At First, only *vc-src--parse-state* used a call to `src status -a` to
inquire about a single FILE. *vc-src-dir-status-files* used
*vc-expand-dirs* to get the status for all files. There was a comment
saying it should be fixed by using one call to `src status -a` instead:

```elisp
(defun vc-src-dir-status-files (dir files update-function)
 ;; FIXME: Use one src status -a call for this
 (if (not files) (setq files (vc-expand-dirs (list dir) 'SRC)))
```

The difference is, that the call to `src status -a` provides a more
detailed status for all files in a directory with just a single call.
This is faster and helps managing the ignore files better.

The first step was to extract the status parser as *vc-src--parse-state*
for reuse in *vc-srv-dir-status-files*:

```elisp
(defun vc-src--parse-state (out)
  (when (null (string-match "does not exist or is unreadable" out))
    (let ((state (aref out 0)))
      (cond
       ;; FIXME: What to do about L code?
       ((eq state ?.) 'up-to-date)
       ((eq state ?A) 'added)
       ((eq state ?M) 'edited)
       ((eq state ?I) 'ignored)
       ((eq state ?R) 'removed)
       ((eq state ?!) 'missing)
       ((eq state ??) 'unregistered)
       (t 'up-to-date)))))
```

The code in *vc-src-state* was replaced by a single call to
*vc-src--parse-state*:

```diff
@@ -163,32 +177,44 @@ For a description of possible values, see `vc-check-master-templates'."
                      "status" "-a" (file-relative-name file))
                   (error nil)))))))
    (when (eq 0 status)
-   [...]
+   (vc-src--parse-state out))))
```

The last diff is a complete refactoring of *vc-src-dir-status-files* to
parse directories recursively, so there is no common code.

The user visible changes are just that unregistered and ignored files
are also shown. Everything else is necessarily identical.

I have attached a new patch with minimal whitespace changes.

[0001-Use-one-src-status-a-call-for-vc-src-dir-status-file.patch (text/x-patch, attachment)]

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 39502 <at> debbugs.gnu.org, dgutov <at> yandex.ru,
 Wolfgang Scherer <Wolfgang.Scherer <at> gmx.de>
Subject: Re: bug#39502: [PATCH] Use one src status -a call for
 vc-src-dir-status-files
Date: Sun, 09 Aug 2020 13:31:23 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> > You posted two versions. Maybe there will be further revisions.
>> I am not planning on any :-). But I am still not sure, which master
>> you mean. I have no write access to the repository.
>
> Can you describe a simple use case to test your patch, and tell what
> are the differences between the situation before and after the patch?
> Then I will try the changes, and install them if I see no problems.

Eli, this was in February.  Wolfgang spun a new version of the patch
without the whitespace changes -- did you find time to test it?

SRC is not a VC I've ever used.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 39502 <at> debbugs.gnu.org, dgutov <at> yandex.ru, Wolfgang.Scherer <at> gmx.de
Subject: Re: bug#39502: [PATCH] Use one src status -a call for
 vc-src-dir-status-files
Date: Sun, 09 Aug 2020 17:13:13 +0300

> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Cc: Wolfgang Scherer <Wolfgang.Scherer <at> gmx.de>,  39502 <at> debbugs.gnu.org,
>   dgutov <at> yandex.ru
> Date: Sun, 09 Aug 2020 13:31:23 +0200
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> > You posted two versions. Maybe there will be further revisions.
> >> I am not planning on any :-). But I am still not sure, which master
> >> you mean. I have no write access to the repository.
> >
> > Can you describe a simple use case to test your patch, and tell what
> > are the differences between the situation before and after the patch?
> > Then I will try the changes, and install them if I see no problems.
> 
> Eli, this was in February.  Wolfgang spun a new version of the patch
> without the whitespace changes -- did you find time to test it?

I tried, but couldn't (for boring reasons unrelated to the patch).  I
don't want to block the changes, so it's okay to install them,
provided that Dmitry agrees.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>, Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 39502 <at> debbugs.gnu.org, Wolfgang.Scherer <at> gmx.de
Subject: Re: bug#39502: [PATCH] Use one src status -a call for
 vc-src-dir-status-files
Date: Sun, 9 Aug 2020 20:42:37 +0300

On 09.08.2020 17:13, Eli Zaretskii wrote:
> I tried, but couldn't (for boring reasons unrelated to the patch).  I
> don't want to block the changes, so it's okay to install them,
> provided that Dmitry agrees.

Now that I've read the code more in-depth, this part looks somewhat 
objectionable:

+                  ;; Ignore all errors.
+                  (condition-case nil

But since the patch only affects SRC (and I don't know why would anybody 
use it), I'm happy to leave the decision to others.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 39502 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Wolfgang.Scherer <at> gmx.de
Subject: Re: bug#39502: [PATCH] Use one src status -a call for
 vc-src-dir-status-files
Date: Sun, 09 Aug 2020 21:50:22 +0200

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

> On 09.08.2020 17:13, Eli Zaretskii wrote:
>> I tried, but couldn't (for boring reasons unrelated to the patch).  I
>> don't want to block the changes, so it's okay to install them,
>> provided that Dmitry agrees.
>
> Now that I've read the code more in-depth, this part looks somewhat
> objectionable:
>
> +                  ;; Ignore all errors.
> +                  (condition-case nil
>
> But since the patch only affects SRC (and I don't know why would
> anybody use it), I'm happy to leave the decision to others.

Well, it's a bit unfortunate to apply a patch that we haven't tested,
but I've now done so anyway.  I did make some white space and stylistic
changes before applying it, though -- Wolfgang, can you check whether I
messed something up there?

But I've pushed the change to Emacs 28.1 (i.e., the Emacs trunk).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Previous Next

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