Re: vc-*-root finctions

[Dan Nicolaescu <dann@ics.uci.edu>]

Thien-Thi Nguyen <ttn@gnuvola.org> writes:

  > () Dan Nicolaescu <dann@ics.uci.edu>
  > () Fri, 22 Feb 2008 07:42:28 -0800
  > 
  >    Because if forces a backend implementor to read two things
  >    instead of one.
  > 
  > Not really.  All you need to know to implement is in the
  > Commentary.  The rationale doesn't add any more information,
  > just design background.

I asked what is KICKP because it's not a familiar predicate name and you
referred to the commentary in the code.

  >    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.
  > 
  > Look at the overall picture.  In the present system, each
  > backend must decide synchronicity, do scratch buffer and
  > subprocess maintenance, compute the result, and do a function
  > call.  

You make it sound like it's a big deal, it's not.

  > This can be error prone (vc-svn-dir-status from vc-svn.el 1.71 leaks
  > buffers, e.g).

Because it's still work in progress, Tom will surely take care of that
when he gets a chance.

  > In the proposed system, the backend need only decide synchronicity
  > and compute its result; vc-status-refresh does the rest.

Looking at this again: the KICKP silliness is completely unnecessary,
you might as well have 2 functions: vc-BACKEND-start-vc-status-command and
vc-BACKEND-process-status-result.  They are 2 different things, why
obfuscate them by putting them in the same function?

The problem is that your scheme breaks if the backend needs to run more
than one process to compute the status.  Like, for example,
vc-git-dir-state does.

When doing the current design I considered putting vc-exec-after in
vc-refresh, but decided not to precisely for that reason: not wanting to
constrain the backend to run a single command.

So yes, the current system where to vc core asks the backend
"compute the status result and tell me when you are done" has some
reasoning behind it.  (And it's nothing new, that how most asynchronous
work gets implemented).

All the synchronicity stuff is completely orthogonal to this.

  > True, vc-status-refresh becomes more complex, but the overall
  > reduction in (redundant) complexity is a win.
  > 
  >    How about way too complex?
  > 
  > Just stick w/ the Commentary, then; that explains the interface
  > requirements.  Lots of things are complex in the core so that
  > the interfaces are simple and regular.  This is one of them.

I disagree that overall model is less complex in your case.

  >    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).
  > 
  > I have tried to explain it but since i don't know where you fail
  > to understand things, i don't know how to explain it better.

This is the first time you explained "why" not only "what", it would
have been a lot easier if you would have done that from the start.  Not
adding so many unrelated issues would have helped too.

I gave you the benefit of the doubt that you might have uncovered some
unseen problem, but not it's clear that you have not.  More, the
proposed design has other core problems.

  > When i fixed the vc-svn.el 1.71 bug, that was when i started to

 What bug is that? Not sure what are you referring to, 1.71 is my commit
 that helps implement a missing feature...