Lars Ingebrigtsen <larsi@HIDDEN>
to control <at> debbugs.gnu.org.
Full text available.Lars Ingebrigtsen <larsi@HIDDEN>
to control <at> debbugs.gnu.org.
Full text available.
Received: (at 7675) by debbugs.gnu.org; 20 Dec 2010 04:11:33 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Sun Dec 19 23:11:33 2010
Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org)
by debbugs.gnu.org with esmtp (Exim 4.69)
(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
id 1PUX5w-0006W1-Rm
for submit <at> debbugs.gnu.org; Sun, 19 Dec 2010 23:11:33 -0500
Received: from rgrjr.com ([216.146.47.5])
by debbugs.gnu.org with esmtp (Exim 4.69)
(envelope-from <rogers-emacs@HIDDEN>) id 1PUX5u-0006Vo-U9
for 7675 <at> debbugs.gnu.org; Sun, 19 Dec 2010 23:11:31 -0500
Received: from rgrjr.dyndns.org (c-66-30-196-77.hsd1.ma.comcast.net
[66.30.196.77])
by rgrjr.com (Postfix on CentOS) with ESMTP id 92FBB160205
for <7675 <at> debbugs.gnu.org>; Mon, 20 Dec 2010 04:17:58 +0000 (UTC)
Received: (qmail 26652 invoked by uid 89); 20 Dec 2010 04:17:58 -0000
Received: from unknown (HELO rgr.rgrjr.com) (192.168.57.1)
by home with SMTP; 20 Dec 2010 04:17:58 -0000
Received: by rgr.rgrjr.com (Postfix, from userid 500)
id 30FC4A4E90; Sun, 19 Dec 2010 23:17:42 -0500 (EST)
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Message-ID: <19726.55525.580236.307665@HIDDEN>
Date: Sun, 19 Dec 2010 23:17:41 -0500
From: Bob Rogers <rogers-emacs@HIDDEN>
To: Stefan Monnier <monnier@HIDDEN>
Subject: Re: bug#7675: 24.0.50; vc-checkin should prompt if the fileset changes
In-Reply-To: <jwvmxo1tbdt.fsf-monnier+emacs@HIDDEN>
References: <19725.10211.399667.389809@HIDDEN>
<jwvzks1rhnl.fsf-monnier+emacs@HIDDEN>
<19726.28388.786955.19167@HIDDEN>
<jwvmxo1tbdt.fsf-monnier+emacs@HIDDEN>
X-Mailer: VM 7.19 under Emacs 24.0.50.1
X-Spam-Score: -3.9 (---)
X-Debbugs-Envelope-To: 7675
Cc: 7675 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.11
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <http://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <http://debbugs.gnu.org/pipermail/debbugs-submit>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <http://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
X-Spam-Score: -3.9 (---)
From: Stefan Monnier <monnier@HIDDEN>
Date: Sun, 19 Dec 2010 21:50:03 -0500
> Thanks for submitting a patch to try and fix this problem.
> A quick obvious issue, tho:
>> --- a/lisp/vc/vc.el
>> +++ b/lisp/vc/vc.el
> [...]
>> +(defun log-edit-deduce-fileset (state-model-only-files)
> If it's in vc.el it can't start with the "log-edit-" prefix.
> Yes, I was wondering about that. I had thought about putting it in
> log-edit.el, but it seems closer to VC internals than log-edit
> internals.
log-edit deals with editing the text buffer, so clearly this job is not
about log-edit but VC.
Fair enough.
> Which would you prefer: Rename it, or move it? If
> "rename", would "vc-log-edit-deduce-fileset" suffice?
I guess so, tho I don't understand why you need this function. IIUC you
should switch to vc-parent before calling vc-deduce-fileset, so that
function never needs to handle log-edit buffers.
Ah, that's a good question. That is what the existing vc-deduce-fileset
does (as I'm sure you know), which means that if you change the *vc-dir*
fileset after starting a commit, C-x v = in the log buffer will diff the
new fileset, but C-c C-c will still commit the old one; this is also a
bug in the pretest [1], and probably back to 23.1.
ISTM that if we want to detect when the *VC-log* buffer and the
*vc-dir* buffer have different filesets, then it makes sense to consider
that the *VC-log* buffer implies a fileset is independent of its parent.
That is the model to which I was coding.
Another way to fix the problem would be to define the existing
vc-deduce-fileset behavior as correct, and fix vc-commit always to
commit the most current fileset. Then you would need a different patch
entirely. I am not crazy about this idea, because it would make it
harder to perform multiple commits in parallel.
A third way to fix it would be to put vc-deduce-fileset in charge of
detecting when the *vc-dir* fileset has changed. In other words, not
only would C-c C-c ask you what was up, C-x v = would as well. This
ISTM would make parallel commits even harder.
> And while I'm thinking of it, a VC fileset really ought to be
> represented by a struct, since all this car-ing and cdr-ing is really
> messy. But filesets are passed to backends, so testing such a change
> would be a pain. What are your thoughts? It is worth it?
It doesn't seem complex enough to warrant such a change.
Maybe another approach is to use pcase.
E.g. instead of
(let ((backend (car fs))
(files (cdr fs)))
..)
or something like that, you'd write
(pcase-let ((`(,backend . ,files) fileset))
...)
Experience with ML-style languages tends to indicate that such pattern
matching tends to reduce the need for named fields,
I was thinking more of abstraction. This sort of destructuring means
you need to know that a fileset is represented as a list in which the
backend comes first and the files come second. When I was trying to
find out how the various backends used the "state" element of that list,
for example, it would have been really nice to have been able to grep
for calls to something like "vc-fileset-state".
tho obviously type checking (and inference) is also an important part
of it since it catches incorrect patterns.
Stefan
Indeed. In fact, the correct pattern for a fileset is more like
`(,backend ,files ,nondir-files ,state ,model) -- so thank you for
illustrating my point. ;-}
I think that is part of why I am less than keen on Haskell and
Erlang, the only two ML-style languages which which I have any
acquaintance.
-- Bob
[1] I hadn't noticed this before because I have a hacked version of
vc-deduce-fileset I wrote more than two years ago that fixes this,
by doing almost the same thing for log buffers as the code in the
patch. Not sure why I didn't report this bug at the time. In any
case, I had forgotten about it completely.
owner <at> debbugs.gnu.org, bug-gnu-emacs@HIDDEN:bug#7675; Package emacs.
Full text available.
Received: (at 7675) by debbugs.gnu.org; 20 Dec 2010 02:43:38 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Sun Dec 19 21:43:38 2010
Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org)
by debbugs.gnu.org with esmtp (Exim 4.69)
(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
id 1PUVir-0004NR-EY
for submit <at> debbugs.gnu.org; Sun, 19 Dec 2010 21:43:37 -0500
Received: from ironport2-out.teksavvy.com ([206.248.154.183]
helo=ironport2-out.pppoe.ca)
by debbugs.gnu.org with esmtp (Exim 4.69)
(envelope-from <monnier@HIDDEN>) id 1PUVip-0004NG-Sh
for 7675 <at> debbugs.gnu.org; Sun, 19 Dec 2010 21:43:36 -0500
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: AvsEAOpSDk1Ld/KP/2dsb2JhbACkLnS8CYVKBIRljhI
X-IronPort-AV: E=Sophos;i="4.60,199,1291611600"; d="scan'208";a="85904688"
Received: from 75-119-242-143.dsl.teksavvy.com (HELO ceviche.home)
([75.119.242.143])
by ironport2-out.pppoe.ca with ESMTP/TLS/ADH-AES256-SHA;
19 Dec 2010 21:50:03 -0500
Received: by ceviche.home (Postfix, from userid 20848)
id 38EDC66112; Sun, 19 Dec 2010 21:50:03 -0500 (EST)
From: Stefan Monnier <monnier@HIDDEN>
To: Bob Rogers <rogers-emacs@HIDDEN>
Subject: Re: bug#7675: 24.0.50; vc-checkin should prompt if the fileset changes
Message-ID: <jwvmxo1tbdt.fsf-monnier+emacs@HIDDEN>
References: <19725.10211.399667.389809@HIDDEN>
<jwvzks1rhnl.fsf-monnier+emacs@HIDDEN>
<19726.28388.786955.19167@HIDDEN>
Date: Sun, 19 Dec 2010 21:50:03 -0500
In-Reply-To: <19726.28388.786955.19167@HIDDEN> (Bob Rogers's message of
"Sun, 19 Dec 2010 15:45:24 -0500")
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
X-Spam-Score: -2.0 (--)
X-Debbugs-Envelope-To: 7675
Cc: 7675 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.11
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <http://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <http://debbugs.gnu.org/pipermail/debbugs-submit>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <http://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
X-Spam-Score: -2.0 (--)
> Thanks for submitting a patch to try and fix this problem.
> A quick obvious issue, tho:
>> --- a/lisp/vc/vc.el
>> +++ b/lisp/vc/vc.el
> [...]
>> +(defun log-edit-deduce-fileset (state-model-only-files)
> If it's in vc.el it can't start with the "log-edit-" prefix.
> Yes, I was wondering about that. I had thought about putting it in
> log-edit.el, but it seems closer to VC internals than log-edit
> internals.
log-edit deals with editing the text buffer, so clearly this job is not
about log-edit but VC.
> Which would you prefer: Rename it, or move it? If
> "rename", would "vc-log-edit-deduce-fileset" suffice?
I guess so, tho I don't understand why you need this function. IIUC you
should switch to vc-parent before calling vc-deduce-fileset, so that
function never needs to handle log-edit buffers.
> And while I'm thinking of it, a VC fileset really ought to be
> represented by a struct, since all this car-ing and cdr-ing is really
> messy. But filesets are passed to backends, so testing such a change
> would be a pain. What are your thoughts? It is worth it?
It doesn't seem complex enough to warrant such a change.
Maybe another approach is to use pcase.
E.g. instead of
(let ((backend (car fs))
(files (cdr fs)))
..)
or something like that, you'd write
(pcase-let ((`(,backend . ,files) fileset))
...)
Experience with ML-style languages tends to indicate that such pattern
matching tends to reduce the need for named fields, tho obviously type
checking (and inference) is also an important part of it since it
catches incorrect patterns.
Stefan
owner <at> debbugs.gnu.org, bug-gnu-emacs@HIDDEN:bug#7675; Package emacs.
Full text available.Received: (at 7675) by debbugs.gnu.org; 19 Dec 2010 20:39:05 +0000 From debbugs-submit-bounces <at> debbugs.gnu.org Sun Dec 19 15:39:04 2010 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>) id 1PUQ24-0004uU-4D for submit <at> debbugs.gnu.org; Sun, 19 Dec 2010 15:39:04 -0500 Received: from rgrjr.com ([216.146.47.5]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from <rogers-emacs@HIDDEN>) id 1PUQ22-0004u1-7X for 7675 <at> debbugs.gnu.org; Sun, 19 Dec 2010 15:39:03 -0500 Received: from rgrjr.dyndns.org (c-66-30-196-77.hsd1.ma.comcast.net [66.30.196.77]) by rgrjr.com (Postfix on CentOS) with ESMTP id 0BC861601AD for <7675 <at> debbugs.gnu.org>; Sun, 19 Dec 2010 20:45:29 +0000 (UTC) Received: (qmail 10121 invoked by uid 89); 19 Dec 2010 20:45:28 -0000 Received: from unknown (HELO rgr.rgrjr.com) (192.168.57.1) by home with SMTP; 19 Dec 2010 20:45:28 -0000 Received: by rgr.rgrjr.com (Postfix, from userid 500) id 1ABFAA4E90; Sun, 19 Dec 2010 15:45:25 -0500 (EST) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <19726.28388.786955.19167@HIDDEN> Date: Sun, 19 Dec 2010 15:45:24 -0500 From: Bob Rogers <rogers-emacs@HIDDEN> To: Stefan Monnier <monnier@HIDDEN> Subject: Re: bug#7675: 24.0.50; vc-checkin should prompt if the fileset changes In-Reply-To: <jwvzks1rhnl.fsf-monnier+emacs@HIDDEN> References: <19725.10211.399667.389809@HIDDEN> <jwvzks1rhnl.fsf-monnier+emacs@HIDDEN> X-Mailer: VM 7.19 under Emacs 24.0.50.1 X-Spam-Score: -4.1 (----) X-Debbugs-Envelope-To: 7675 Cc: 7675 <at> debbugs.gnu.org X-BeenThere: debbugs-submit <at> debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: <debbugs-submit.debbugs.gnu.org> List-Unsubscribe: <http://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe> List-Archive: <http://debbugs.gnu.org/pipermail/debbugs-submit> List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org> List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help> List-Subscribe: <http://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe> Sender: debbugs-submit-bounces <at> debbugs.gnu.org Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org X-Spam-Score: -4.0 (----) From: Stefan Monnier <monnier@HIDDEN> Date: Sun, 19 Dec 2010 08:57:00 -0500 Thanks for submitting a patch to try and fix this problem. A quick obvious issue, tho: > --- a/lisp/vc/vc.el > +++ b/lisp/vc/vc.el [...] > +(defun log-edit-deduce-fileset (state-model-only-files) If it's in vc.el it can't start with the "log-edit-" prefix. Yes, I was wondering about that. I had thought about putting it in log-edit.el, but it seems closer to VC internals than log-edit internals. Which would you prefer: Rename it, or move it? If "rename", would "vc-log-edit-deduce-fileset" suffice? Stefan "who hasn't yet had time to look at the rest of the patch" Just as well, considering that the patch fails for single-file commits. I am thinking that vc-commit only ever needs to re-examine the fileset if the commit was (a) started in *vc-dir* and (b) with marked files. Otherwise, the fileset is determined by the way C-x v v is invoked, and there's no way to change it later. True? And while I'm thinking of it, a VC fileset really ought to be represented by a struct, since all this car-ing and cdr-ing is really messy. But filesets are passed to backends, so testing such a change would be a pain. What are your thoughts? It is worth it? -- Bob
owner <at> debbugs.gnu.org, bug-gnu-emacs@HIDDEN:bug#7675; Package emacs.
Full text available.
Received: (at 7675) by debbugs.gnu.org; 19 Dec 2010 13:50:42 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Sun Dec 19 08:50:42 2010
Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org)
by debbugs.gnu.org with esmtp (Exim 4.69)
(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
id 1PUJen-00044n-4V
for submit <at> debbugs.gnu.org; Sun, 19 Dec 2010 08:50:42 -0500
Received: from ironport2-out.teksavvy.com ([206.248.154.181]
helo=ironport2-out.pppoe.ca)
by debbugs.gnu.org with esmtp (Exim 4.69)
(envelope-from <monnier@HIDDEN>) id 1PUJek-00044Y-6d
for 7675 <at> debbugs.gnu.org; Sun, 19 Dec 2010 08:50:34 -0500
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: AvsEADeeDU1Ld/KP/2dsb2JhbACkM3S6cIVKBIRljhI
X-IronPort-AV: E=Sophos;i="4.60,196,1291611600"; d="scan'208";a="85871610"
Received: from 75-119-242-143.dsl.teksavvy.com (HELO pastel.home)
([75.119.242.143])
by ironport2-out.pppoe.ca with ESMTP/TLS/ADH-AES256-SHA;
19 Dec 2010 08:57:00 -0500
Received: by pastel.home (Postfix, from userid 20848)
id 567EF58E99; Sun, 19 Dec 2010 08:57:00 -0500 (EST)
From: Stefan Monnier <monnier@HIDDEN>
To: Bob Rogers <rogers-emacs@HIDDEN>
Subject: Re: bug#7675: 24.0.50; vc-checkin should prompt if the fileset changes
Message-ID: <jwvzks1rhnl.fsf-monnier+emacs@HIDDEN>
References: <19725.10211.399667.389809@HIDDEN>
Date: Sun, 19 Dec 2010 08:57:00 -0500
In-Reply-To: <19725.10211.399667.389809@HIDDEN> (Bob Rogers's message
of "Sat, 18 Dec 2010 16:30:11 -0500")
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
X-Spam-Score: -2.0 (--)
X-Debbugs-Envelope-To: 7675
Cc: 7675 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.11
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <http://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <http://debbugs.gnu.org/pipermail/debbugs-submit>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <http://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
X-Spam-Score: -2.0 (--)
Thanks for submitting a patch to try and fix this problem.
A quick obvious issue, tho:
> --- a/lisp/vc/vc.el
> +++ b/lisp/vc/vc.el
[...]
> +(defun log-edit-deduce-fileset (state-model-only-files)
If it's in vc.el it can't start with the "log-edit-" prefix.
Stefan "who hasn't yet had time to look at the rest of the patch"
owner <at> debbugs.gnu.org, bug-gnu-emacs@HIDDEN:bug#7675; Package emacs.
Full text available.Received: (at 7675) by debbugs.gnu.org; 19 Dec 2010 04:39:10 +0000 From debbugs-submit-bounces <at> debbugs.gnu.org Sat Dec 18 23:39:09 2010 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>) id 1PUB37-00086z-IN for submit <at> debbugs.gnu.org; Sat, 18 Dec 2010 23:39:09 -0500 Received: from rgrjr.com ([216.146.47.5]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from <rogers-emacs@HIDDEN>) id 1PUB35-00086V-5F for 7675 <at> debbugs.gnu.org; Sat, 18 Dec 2010 23:39:07 -0500 Received: from rgrjr.dyndns.org (c-66-30-196-77.hsd1.ma.comcast.net [66.30.196.77]) by rgrjr.com (Postfix on CentOS) with ESMTP id 3AEE81600DE for <7675 <at> debbugs.gnu.org>; Sun, 19 Dec 2010 04:45:32 +0000 (UTC) Received: (qmail 11379 invoked by uid 89); 19 Dec 2010 04:45:31 -0000 Received: from unknown (HELO rgr.rgrjr.com) (192.168.57.1) by home with SMTP; 19 Dec 2010 04:45:31 -0000 Received: by rgr.rgrjr.com (Postfix, from userid 500) id 027F2A4E6E; Sat, 18 Dec 2010 23:45:16 -0500 (EST) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <19725.36316.354969.581382@HIDDEN> Date: Sat, 18 Dec 2010 23:45:16 -0500 From: Bob Rogers <rogers-emacs@HIDDEN> To: 7675 <at> debbugs.gnu.org Subject: bug#7675: 24.0.50; vc-checkin should prompt if the fileset changes In-Reply-To: <handler.7675.B.129270743922452.ack <at> debbugs.gnu.org> References: <19725.10211.399667.389809@HIDDEN> <handler.7675.B.129270743922452.ack <at> debbugs.gnu.org> X-Mailer: VM 7.19 under Emacs 24.0.50.1 X-Spam-Score: -4.3 (----) X-Debbugs-Envelope-To: 7675 X-BeenThere: debbugs-submit <at> debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: <debbugs-submit.debbugs.gnu.org> List-Unsubscribe: <http://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe> List-Archive: <http://debbugs.gnu.org/pipermail/debbugs-submit> List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org> List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help> List-Subscribe: <http://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe> Sender: debbugs-submit-bounces <at> debbugs.gnu.org Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org X-Spam-Score: -4.2 (----) Oops; please hold off on this. It seems that this patch does the wrong thing if the commit is not started from *vc-dir*, or started on a single unmarked file. -- Bob
owner <at> debbugs.gnu.org, bug-gnu-emacs@HIDDEN:bug#7675; Package emacs.
Full text available.Glenn Morris <rgm@HIDDEN>
to control <at> debbugs.gnu.org.
Full text available.Glenn Morris <rgm@HIDDEN>
to control <at> debbugs.gnu.org.
Full text available.
Received: (at submit) by debbugs.gnu.org; 18 Dec 2010 21:23:59 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Sat Dec 18 16:23:59 2010
Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org)
by debbugs.gnu.org with esmtp (Exim 4.69)
(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
id 1PU4Fy-0005q5-Ij
for submit <at> debbugs.gnu.org; Sat, 18 Dec 2010 16:23:59 -0500
Received: from eggs.gnu.org ([140.186.70.92])
by debbugs.gnu.org with esmtp (Exim 4.69)
(envelope-from <rogers-emacs@HIDDEN>) id 1PU4Fv-0005ps-4d
for submit <at> debbugs.gnu.org; Sat, 18 Dec 2010 16:23:56 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
(envelope-from <rogers-emacs@HIDDEN>) id 1PU4M6-0004gb-Id
for submit <at> debbugs.gnu.org; Sat, 18 Dec 2010 16:30:20 -0500
X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on eggs.gnu.org
X-Spam-Level:
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00
autolearn=unavailable version=3.3.1
Received: from lists.gnu.org ([199.232.76.165]:46727)
by eggs.gnu.org with esmtp (Exim 4.71)
(envelope-from <rogers-emacs@HIDDEN>) id 1PU4M6-0004gV-Av
for submit <at> debbugs.gnu.org; Sat, 18 Dec 2010 16:30:18 -0500
Received: from [140.186.70.92] (port=40961 helo=eggs.gnu.org)
by lists.gnu.org with esmtp (Exim 4.43) id 1PU4M4-00038A-Ff
for bug-gnu-emacs@HIDDEN; Sat, 18 Dec 2010 16:30:18 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
(envelope-from <rogers-emacs@HIDDEN>) id 1PU4M2-0004fS-IY
for bug-gnu-emacs@HIDDEN; Sat, 18 Dec 2010 16:30:16 -0500
Received: from rgrjr.com ([216.146.47.5]:35197)
by eggs.gnu.org with esmtp (Exim 4.71)
(envelope-from <rogers-emacs@HIDDEN>) id 1PU4M2-0004fA-9M
for bug-gnu-emacs@HIDDEN; Sat, 18 Dec 2010 16:30:14 -0500
Received: from rgrjr.dyndns.org (c-66-30-196-77.hsd1.ma.comcast.net
[66.30.196.77])
by rgrjr.com (Postfix on CentOS) with ESMTP id 905981601CA
for <bug-gnu-emacs@HIDDEN>; Sat, 18 Dec 2010 21:30:12 +0000 (UTC)
Received: (qmail 27398 invoked by uid 89); 18 Dec 2010 21:30:12 -0000
Received: from unknown (HELO rgr.rgrjr.com) (192.168.57.1)
by home with SMTP; 18 Dec 2010 21:30:12 -0000
Received: by rgr.rgrjr.com (Postfix, from userid 500)
id C2301A4E6E; Sat, 18 Dec 2010 16:30:11 -0500 (EST)
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="MyecuAFNpv"
Content-Transfer-Encoding: 7bit
Message-ID: <19725.10211.399667.389809@HIDDEN>
Date: Sat, 18 Dec 2010 16:30:11 -0500
From: Bob Rogers <rogers-emacs@HIDDEN>
To: bug-gnu-emacs@HIDDEN
Subject: 24.0.50; vc-checkin should prompt if the fileset changes
X-Mailer: VM 7.19 under Emacs 24.0.50.1
X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3)
X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2)
X-Spam-Score: -6.2 (------)
X-Debbugs-Envelope-To: submit
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.11
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <http://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <http://debbugs.gnu.org/pipermail/debbugs-submit>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <http://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>,
<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
X-Spam-Score: -6.2 (------)
--MyecuAFNpv
Content-Type: text/plain; charset=us-ascii
Content-Description: message body text
Content-Transfer-Encoding: 7bit
If you start a multifile commit from *vc-dir* and then change the
selected fileset, log-edit-done (C-c C-c) commits the original fileset
without comment. The attached patch fixes this by recomputing the
fileset, and prompting the user if anything changed. First, the user is
offered a chance to use the new fileset (since that is probably what was
intended), then to continue the commit with the original fileset. If
both choices are refused, then the commit is aborted. Most of the patch
factors vc-deduce-fileset-internal out of vc-deduce-fileset and
vc-filter-files-to-commit out of vc-next-action in order to avoid code
duplication.
For symmetry, C-x v v ought to make the equivalent check when reusing
an existing log buffer. It currently overwrites the changeset for any
commit in progress. If that change is acceptable, I will submit a
separate patch.
Along the way, I've also changed the name of the "observer" parameter
of vc-deduce-fileset to "nonviolent-p", and documented it as such, since
this seems to be what was intended: This is passed non-nil only from
places that do not change VC state, such as vc-diff and vc-log.
-- Bob Rogers
http://www.rgrjr.com/
--MyecuAFNpv
Content-Type: text/plain
Content-Description:
Content-Disposition: inline;
filename="vc-log-prompt-if-fileset-changed.patch"
Content-Transfer-Encoding: 7bit
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 40f91b7..d55bf48 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -941,33 +941,42 @@ Within directories, only files already under version control are noticed."
(declare-function vc-dir-current-file "vc-dir" ())
(declare-function vc-dir-deduce-fileset "vc-dir" (&optional state-model-only-files))
-(defun vc-deduce-fileset (&optional observer allow-unregistered
- state-model-only-files)
- "Deduce a set of files and a backend to which to apply an operation.
+(defun log-edit-deduce-fileset (state-model-only-files)
+ ;; Attempt to reconstruct the original fileset from the log-edit
+ ;; buffer. [We ought to be able to do a better job. Better still,
+ ;; we ought to be able to return the fileset used to create the
+ ;; buffer. At least we don't need state-model-only-files, since
+ ;; that has already been taken care of. -- rgr, 27-Nov-10.]
+ (let ((backend
+ (and vc-parent-buffer
+ (with-current-buffer vc-parent-buffer
+ (if (derived-mode-p 'vc-dir-mode)
+ vc-dir-backend
+ (vc-responsible-backend
+ (or buffer-file-name default-directory)))))))
+ (and backend
+ (list backend vc-log-fileset))))
+
+(defun vc-deduce-fileset-internal (&optional nonviolent-p state-model-only-files)
+ "Deduce a set of registered files and a backend for an operation.
Return (BACKEND FILESET FILESET-ONLY-FILES STATE CHECKOUT-MODEL).
-If we're in VC-dir mode, the fileset is the list of marked files.
-Otherwise, if we're looking at a buffer visiting a version-controlled file,
-the fileset is a singleton containing this file.
-If none of these conditions is met, but ALLOW_UNREGISTERED is on and the
-visited file is not registered, return a singleton fileset containing it.
-Otherwise, throw an error.
+See vc-deduce-fileset for details; we do just the first part of the search,
+looking for registered files, returning nil if nothing found.
-STATE-MODEL-ONLY-FILES if non-nil, means that the caller needs
-the FILESET-ONLY-FILES STATE and MODEL info. Otherwise, that
-part may be skipped.
-BEWARE: this function may change the
-current buffer."
- ;; FIXME: OBSERVER is unused. The name is not intuitive and is not
- ;; documented. It's set to t when called from diff and print-log.
+BEWARE: this function may change the current buffer."
(let (backend)
(cond
((derived-mode-p 'vc-dir-mode)
(vc-dir-deduce-fileset state-model-only-files))
((derived-mode-p 'dired-mode)
- (if observer
+ (if nonviolent-p
(vc-dired-deduce-fileset)
(error "State changing VC operations not supported in `dired-mode'")))
+ ((derived-mode-p 'log-edit-mode)
+ ;; This has a vc-parent-buffer, but that might result in a
+ ;; different fileset.
+ (log-edit-deduce-fileset state-model-only-files))
((setq backend (vc-backend buffer-file-name))
(if state-model-only-files
(list backend (list buffer-file-name)
@@ -978,11 +987,37 @@ current buffer."
((and (buffer-live-p vc-parent-buffer)
;; FIXME: Why this test? --Stef
(or (buffer-file-name vc-parent-buffer)
- (with-current-buffer vc-parent-buffer
- (derived-mode-p 'vc-dir-mode))))
+ (with-current-buffer vc-parent-buffer
+ (derived-mode-p 'vc-dir-mode))))
+ ;; Note that vc-parent-buffer must be registered.
(progn ;FIXME: Why not `with-current-buffer'? --Stef.
(set-buffer vc-parent-buffer)
- (vc-deduce-fileset observer allow-unregistered state-model-only-files)))
+ (vc-deduce-fileset-internal nonviolent-p state-model-only-files))))))
+
+(defun vc-deduce-fileset (&optional nonviolent-p allow-unregistered
+ state-model-only-files)
+ "Deduce a set of files and a backend to which to apply an operation.
+
+Return (BACKEND FILESET FILESET-ONLY-FILES STATE CHECKOUT-MODEL).
+If we're in VC-dir mode, the fileset is the list of marked files.
+Otherwise, if we're in dired-mode, use current/marked files.
+Otherwise, if we're looking at a buffer visiting a version-controlled file,
+the fileset is a singleton containing this file.
+Otherwise, if we're in a VC buffer that has a parent, try again in the parent.
+If none of these conditions is met, but ALLOW-UNREGISTERED is true and the
+visited file is not registered, return a singleton fileset containing it.
+Otherwise, throw an error.
+
+NONVIOLENT-P means that the fileset will be used for a non-state-changing
+operation, such as vc-log or vc-diff.
+
+STATE-MODEL-ONLY-FILES if non-nil, means that the caller needs
+the FILESET-ONLY-FILES STATE and MODEL info. Otherwise, that
+part may be skipped.
+BEWARE: this function may change the
+current buffer."
+ (cond
+ ((vc-deduce-fileset-internal nonviolent-p state-model-only-files))
((not buffer-file-name)
(error "Buffer %s is not associated with a file" (buffer-name)))
((and allow-unregistered (not (vc-registered buffer-file-name)))
@@ -994,7 +1029,7 @@ current buffer."
nil)
(list (vc-backend-for-registration (buffer-file-name))
(list buffer-file-name))))
- (t (error "No fileset is available here")))))
+ (t (error "No fileset is available here"))))
(defun vc-dired-deduce-fileset ()
(let ((backend (vc-responsible-backend default-directory)))
@@ -1036,6 +1071,42 @@ current buffer."
(eq p q)
(and (member p '(edited added removed)) (member q '(edited added removed)))))
+(defun vc-filter-files-to-commit (fileset)
+ ;; Given a fileset, return those that are ready to commit.
+ (let* ((files (nth 1 fileset))
+ (model (nth 4 fileset))
+ (ready-for-commit files))
+ ;; If files are edited but read-only, give user a chance to correct
+ (dolist (file files)
+ (unless (file-writable-p file)
+ ;; Make the file+buffer read-write.
+ (unless (y-or-n-p (format "%s is edited but read-only; make it writable and continue?" file))
+ (error "Aborted"))
+ (set-file-modes file (logior (file-modes file) 128))
+ (let ((visited (get-file-buffer file)))
+ (when visited
+ (with-current-buffer visited
+ (toggle-read-only -1))))))
+ ;; Allow user to revert files with no changes.
+ ;; [shouldn't we factor (not (eq model 'implicit)) out of the loop?
+ ;; -- rgr, 26-Nov-10.]
+ (save-excursion
+ (dolist (file files)
+ (let ((visited (get-file-buffer file)))
+ ;; For files with locking, if the file does not contain
+ ;; any changes, just let go of the lock, i.e. revert.
+ (when (and (not (eq model 'implicit))
+ (vc-workfile-unchanged-p file)
+ ;; If buffer is modified, that means the user just
+ ;; said no to saving it; in that case, don't revert,
+ ;; because the user might intend to save after
+ ;; finishing the log entry and committing.
+ (not (and visited (buffer-modified-p))))
+ (vc-revert-file file)
+ (setq ready-for-commit (delete file ready-for-commit))))))
+ ;; Remaining files need to be committed
+ ready-for-commit))
+
;; Here's the major entry point.
;;;###autoload
@@ -1112,34 +1183,7 @@ merge in the changes into your working copy."
(message "Fileset is up-to-date"))))
;; Files have local changes
((vc-compatible-state state 'edited)
- (let ((ready-for-commit files))
- ;; If files are edited but read-only, give user a chance to correct
- (dolist (file files)
- (unless (file-writable-p file)
- ;; Make the file+buffer read-write.
- (unless (y-or-n-p (format "%s is edited but read-only; make it writable and continue?" file))
- (error "Aborted"))
- (set-file-modes file (logior (file-modes file) 128))
- (let ((visited (get-file-buffer file)))
- (when visited
- (with-current-buffer visited
- (toggle-read-only -1))))))
- ;; Allow user to revert files with no changes
- (save-excursion
- (dolist (file files)
- (let ((visited (get-file-buffer file)))
- ;; For files with locking, if the file does not contain
- ;; any changes, just let go of the lock, i.e. revert.
- (when (and (not (eq model 'implicit))
- (vc-workfile-unchanged-p file)
- ;; If buffer is modified, that means the user just
- ;; said no to saving it; in that case, don't revert,
- ;; because the user might intend to save after
- ;; finishing the log entry and committing.
- (not (and visited (buffer-modified-p))))
- (vc-revert-file file)
- (setq ready-for-commit (delete file ready-for-commit))))))
- ;; Remaining files need to be committed
+ (let ((ready-for-commit (vc-filter-files-to-commit vc-fileset)))
(if (not ready-for-commit)
(message "No files remain to be committed")
(if (not verbose)
@@ -1387,6 +1431,39 @@ Type \\[vc-next-action] to check in changes.")
".\n")
(message "Please explain why you stole the lock. Type C-c C-c when done.")))
+(defun vc-confirm-files-if-changed (old-files new-files)
+ ;; Given two lists of file names, return t if they are the same, the
+ ;; symbol confirmed if the user says to check in the new set, nil if
+ ;; the user says to use the old set, and throw an error otherwise.
+ (let ((removed-files nil)
+ (added-files nil))
+ ;; Compute the difference sets.
+ (dolist (old-file old-files)
+ (unless (member old-file new-files)
+ (push old-file removed-files)))
+ (dolist (new-file new-files)
+ (unless (member new-file old-files)
+ (push new-file added-files)))
+ (cond ((and (null removed-files) (null added-files))
+ ;; No change.
+ t)
+ ((let ((added-line
+ (if added-files
+ (concat "\n Added: " (vc-delistify added-files))
+ ""))
+ (removed-line
+ (if removed-files
+ (concat "\n Removed: " (vc-delistify removed-files))
+ "")))
+ (yes-or-no-p (concat "Fileset has changed:"
+ added-line removed-line
+ "\nUse the new fileset? ")))
+ 'confirmed)
+ ((yes-or-no-p "Continue anyway? ")
+ nil)
+ (t
+ (error "Checkin aborted.")))))
+
(defun vc-checkin (files backend &optional rev comment initial-contents)
"Check in FILES.
The optional argument REV may be a string specifying the new revision
@@ -1411,6 +1488,12 @@ Runs the normal hooks `vc-before-checkin-hook' and `vc-checkin-hook'."
(vc-call-backend backend 'log-edit-mode))
(lexical-let ((rev rev))
(lambda (files comment)
+ ;; Check to see if the fileset has changed.
+ (let ((new (vc-filter-files-to-commit (vc-deduce-fileset-internal))))
+ (when (vc-confirm-files-if-changed files new)
+ (setq files new)
+ ;; Apparently, this is needed to update the right fileset.
+ (setq vc-log-files new)))
(message "Checking in %s..." (vc-delistify files))
;; "This log message intentionally left almost blank".
;; RCS 5.7 gripes about white-space-only comments too.
--MyecuAFNpv--
Bob Rogers <rogers-emacs@HIDDEN>:bug-gnu-emacs@HIDDEN.
Full text available.owner <at> debbugs.gnu.org, bug-gnu-emacs@HIDDEN:bug#7675; Package emacs.
Full text available.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997 nCipher Corporation Ltd,
1994-97 Ian Jackson.