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

[Justin Bogner <mail@justinbogner.com>]

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.
>
>
>         Stefan
>

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.