From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Thien-Thi Nguyen Newsgroups: gmane.emacs.devel Subject: Re: vc-*-root finctions Date: Fri, 22 Feb 2008 15:41:22 +0100 Message-ID: <87ablt3vfx.fsf@ambire.localdomain> References: <87skzn3mq9.fsf@ambire.localdomain> <87mypvxzd2.fsf@ambire.localdomain> <200802201850.m1KIofIk000198@sallyv1.ics.uci.edu> <87tzk2xr2q.fsf@ambire.localdomain> <200802211835.m1LIZcZ5005055@sallyv1.ics.uci.edu> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1203691531 5845 80.91.229.12 (22 Feb 2008 14:45:31 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 22 Feb 2008 14:45:31 +0000 (UTC) Cc: emacs-devel@gnu.org To: Dan Nicolaescu Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Feb 22 15:45:54 2008 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.50) id 1JSZ9d-0001ZN-V9 for ged-emacs-devel@m.gmane.org; Fri, 22 Feb 2008 15:45:38 +0100 Original-Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JSZ98-0004TE-Iv for ged-emacs-devel@m.gmane.org; Fri, 22 Feb 2008 09:45:06 -0500 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1JSZ7Z-0003WJ-GQ for emacs-devel@gnu.org; Fri, 22 Feb 2008 09:43:29 -0500 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1JSZ7X-0003V5-JT for emacs-devel@gnu.org; Fri, 22 Feb 2008 09:43:29 -0500 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JSZ7X-0003Uy-GN for emacs-devel@gnu.org; Fri, 22 Feb 2008 09:43:27 -0500 Original-Received: from [151.61.143.248] (helo=ambire.localdomain) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1JSZ7W-0007U6-TW for emacs-devel@gnu.org; Fri, 22 Feb 2008 09:43:27 -0500 Original-Received: from ttn by ambire.localdomain with local (Exim 4.63) (envelope-from ) id 1JSZ5X-0002Yy-0l; Fri, 22 Feb 2008 15:41:23 +0100 In-Reply-To: <200802211835.m1LIZcZ5005055@sallyv1.ics.uci.edu> (Dan Nicolaescu's message of "Thu, 21 Feb 2008 10:35:38 -0800") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.0.60 (gnu/linux) X-detected-kernel: by monty-python.gnu.org: Genre and OS details not recognized. X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:89966 Archived-At: () Dan Nicolaescu () Thu, 21 Feb 2008 10:35:38 -0800 It seems that this patch contains multiple independent things. It would be better to split them, so they can be judged separately. Good point. Will do so in the next few days. As a documentation stickler you should know that having a backend implementer look for documentation in both the beginning of vc.el and in the comment in vc-status-refresh is not a good idea. Why is not a good idea? One explains the interface, the other the rationale. More, the comment in vc-status-refresh talks about "We used to do this:" is just pointless. Lots of comments in Emacs are likewise pointless, i suppose. In any case, i'll move it to top-level and make it less time-oriented. Frankly, having a function that is called twice once with an argument t and another time with nil, and does completely different things depending on the argument just screams WEIRD. Better have 2 different functions. I disagree. More functions is more maintenance. The fact that there's a need for about 70 lines of comments for one backend function, points to some design problems. How so? In general, it hard to see what you are trying to accomplish here, and why you changes are better than what is there now. The changes try to reduce failure modes, which, being "potential" rather than actual issues, may indeed be difficult to appreciate. You haven't explained why it is better to introduce the synchronous behavior when I said that it can be easily used with the current API. Well, i haven't introduced synchronous behavior in vc.el, so why would i want to explain that? Which would be wrong. git might be fast, but it takes a long time if it has to read the inodes from disk or NFS on a big tree (which happens every morning or after a big compilation job). I don't know about that. I observe that "git status" on emacs.git takes a little longer (maybe five seconds) the first time it is run and then less than a second for subsequent times. (Perhaps you're not talking about "git status" that vc-git-dir-status uses?) > Yes, but removing the need to specify UPDATE-FUNCTION is better. Why is that a big problem. More functions, more maintenance: It's not a big problem when used correctly; it's a big problem when misunderstood and subsequently used incorrectly. Occam's razor and all that. Yeah, this was intended to call a backend specific function at some point, so better keep it a separate function that mix this code in the middle of something else. Regardless, the best place is during vc-status-refresh. +(defsubst vc-overview-p () ...) There are a few other places that could use this. But this should go at the time vc-dired is completely replaced. Why not check this in separately? Good idea. I will commit it in the next day or two. In the meantime, interested git users can look at: http://www.gnuvola.org/wip/ (vc-status-hacking) I will henceforth be munging there instead of posting patches. thi