[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [patch] vc-find-root with invert

From: Justin Bogner
Subject: Re: [patch] vc-find-root with invert
Date: Mon, 21 Jul 2008 15:33:54 -0600
User-agent: Thunderbird (X11/20080505)

Stefan Monnier wrote:
Reading the docstring, my guess would be: vc-find-root walks the dir
tree up _until_ it finds WITNESS (as an evidence for the VC tree's root
dir). Unless we have CVS or SVN or similar, where every subdir has
WITNESS, so the root would be the last up-tree directory having WITNESS.

That still doesn't tell me what it does.  I mean I can read the code and
understand what is the effect of calling the code like this, but I have
no idea what effect it has for the user.  AFAICT it's a misfeature.

This is exactly correct, though in the current trunk there is a bug
such that the function does not behave like this. The patch causes it
to work as documented.

I think we should remove this misfeature rather than fix it.


There are two types of version control directory, ones with a "root"
directory in each directory under version control, and on with a
"root" directory in the root of the version control. Since this
function's purpose is to find the actual root of the tree, the latter
case is simple, where it needs only find the "root" directory.

However, for the other case, we need traverse upwards until we don't
find the directory, returning the last one that does. As it stands, if
we don't find the directory we look, we return wherever we started
searching, which is incorrect.

As far as getting rid of invert, we could do that, and the function
would then return something more reasonable than it does now, but it
wouldn't actually find the root unless you happened to try the root
first. Even so, this would make it work slightly better in some cases
(at least it would be `nil' when it should be). It would also consider
subdirectories that weren't in CVS to be part of CVS, which is probably
wrong, but possibly not common (I don't know about this).

I'm not convinced that `invert' is a misfeature, as it is certainly a
good intention. However, if you are convinced that it's not useful,
then I can come up with a patch that removes it, since I would prefer
not having it to having a broken version as it is now.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]