From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Bob Rogers Newsgroups: gmane.emacs.bugs Subject: bug#7675: 24.0.50; vc-checkin should prompt if the fileset changes Date: Sun, 19 Dec 2010 23:17:41 -0500 Message-ID: <19726.55525.580236.307665@rgr.rgrjr.com> References: <19725.10211.399667.389809@rgr.rgrjr.com> <19726.28388.786955.19167@rgr.rgrjr.com> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-Trace: dough.gmane.org 1292818984 21022 80.91.229.12 (20 Dec 2010 04:23:04 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Mon, 20 Dec 2010 04:23:04 +0000 (UTC) Cc: 7675@debbugs.gnu.org To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Mon Dec 20 05:23:00 2010 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1PUXH1-0007zm-Om for geb-bug-gnu-emacs@m.gmane.org; Mon, 20 Dec 2010 05:23:00 +0100 Original-Received: from localhost ([127.0.0.1]:35323 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PUXH0-0006vV-VE for geb-bug-gnu-emacs@m.gmane.org; Sun, 19 Dec 2010 23:22:59 -0500 Original-Received: from [140.186.70.92] (port=33357 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PUXGv-0006ox-L3 for bug-gnu-emacs@gnu.org; Sun, 19 Dec 2010 23:22:54 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PUXGt-00048x-D9 for bug-gnu-emacs@gnu.org; Sun, 19 Dec 2010 23:22:53 -0500 Original-Received: from debbugs.gnu.org ([140.186.70.43]:49127) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PUXGt-00048s-6u for bug-gnu-emacs@gnu.org; Sun, 19 Dec 2010 23:22:51 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.69) (envelope-from ) id 1PUX6Q-0006Wi-7p; Sun, 19 Dec 2010 23:12:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Bob Rogers Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-To: owner@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 20 Dec 2010 04:12:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 7675 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 7675-submit@debbugs.gnu.org id=B7675.129281829325052 (code B ref 7675); Mon, 20 Dec 2010 04:12:02 +0000 Original-Received: (at 7675) by debbugs.gnu.org; 20 Dec 2010 04:11:33 +0000 Original-Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PUX5w-0006W1-Rm for submit@debbugs.gnu.org; Sun, 19 Dec 2010 23:11:33 -0500 Original-Received: from rgrjr.com ([216.146.47.5]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PUX5u-0006Vo-U9 for 7675@debbugs.gnu.org; Sun, 19 Dec 2010 23:11:31 -0500 Original-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@debbugs.gnu.org>; Mon, 20 Dec 2010 04:17:58 +0000 (UTC) Original-Received: (qmail 26652 invoked by uid 89); 20 Dec 2010 04:17:58 -0000 Original-Received: from unknown (HELO rgr.rgrjr.com) (192.168.57.1) by home with SMTP; 20 Dec 2010 04:17:58 -0000 Original-Received: by rgr.rgrjr.com (Postfix, from userid 500) id 30FC4A4E90; Sun, 19 Dec 2010 23:17:42 -0500 (EST) In-Reply-To: X-Mailer: VM 7.19 under Emacs 24.0.50.1 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list Resent-Date: Sun, 19 Dec 2010 23:12:02 -0500 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:42651 Archived-At: From: Stefan Monnier 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.