From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Dan Nicolaescu Newsgroups: gmane.emacs.devel Subject: Re: vc-*-root finctions Date: Fri, 22 Feb 2008 07:42:28 -0800 Message-ID: <200802221542.m1MFgSre027158@sallyv1.ics.uci.edu> 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> <87ablt3vfx.fsf@ambire.localdomain> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1203695040 19343 80.91.229.12 (22 Feb 2008 15:44:00 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 22 Feb 2008 15:44:00 +0000 (UTC) Cc: emacs-devel@gnu.org To: Thien-Thi Nguyen Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Feb 22 16:44:24 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 1JSa48-0007l3-OP for ged-emacs-devel@m.gmane.org; Fri, 22 Feb 2008 16:44:01 +0100 Original-Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JSa3d-0000SZ-BC for ged-emacs-devel@m.gmane.org; Fri, 22 Feb 2008 10:43:29 -0500 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1JSa3Y-0000Qp-I1 for emacs-devel@gnu.org; Fri, 22 Feb 2008 10:43:24 -0500 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1JSa3X-0000PP-ER for emacs-devel@gnu.org; Fri, 22 Feb 2008 10:43:23 -0500 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JSa3X-0000PB-BW for emacs-devel@gnu.org; Fri, 22 Feb 2008 10:43:23 -0500 Original-Received: from sallyv1.ics.uci.edu ([128.195.1.109]) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_3DES_EDE_CBC_SHA1:24) (Exim 4.60) (envelope-from ) id 1JSa3W-0000r9-QE for emacs-devel@gnu.org; Fri, 22 Feb 2008 10:43:23 -0500 X-ICS-MailScanner-Watermark: 1204299749.11789@Q46zd+wqIIw2PAU9DXcaQg Original-Received: from mothra.ics.uci.edu (mothra.ics.uci.edu [128.195.6.93]) by sallyv1.ics.uci.edu (8.13.7+Sun/8.13.7) with ESMTP id m1MFgSre027158; Fri, 22 Feb 2008 07:42:28 -0800 (PST) In-Reply-To: <87ablt3vfx.fsf@ambire.localdomain> (Thien-Thi Nguyen's message of "Fri, 22 Feb 2008 15:41:22 +0100") Original-Lines: 105 X-ICS-MailScanner: Found to be clean X-ICS-MailScanner-SpamCheck: not spam, SpamAssassin (score=-1.44, required 5, autolearn=disabled, ALL_TRUSTED -1.44) X-ICS-MailScanner-From: dann@mothra.ics.uci.edu X-detected-kernel: by monty-python.gnu.org: Solaris 10 (beta) 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:89973 Archived-At: Thien-Thi Nguyen writes: > () 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. Because if forces a backend implementor to read two things instead of one. > 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. Well, you are effectively introducing two functions, but squeezing them into one with a very weird interface. Explain how is that allows for less 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? How about way too complex? > 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. Can you please explain what you mean rather than hand wave? Given that you are replacing existing working code, it would be nice if you'd explain what problem you are trying to solve, and why the approach you took is the right way to solve the problem (other than applying the NIH principle). > 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? You are changing code that is simple to something that is strange and complex. Enquiring minds would like to know the reasoning behind those changes and to make sure that the changes you are making are the right way to solve whatever problem you are trying to solve. > 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 That's five seconds on a local disk, right? Put that on an busy NFS server, and do it for a bigger tree than emacs and those five seconds go up really fast. > > 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. Occam's razor also applies to functions with terribly complicated interfaces. So no, the above is not a good motivation. > 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. And why is it better to not have a separate function? > +(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. As you wish. But please don't commit anything that changes the way vc-status works before posting it here.