[MERGE] fix add in view-aware trees

[Robert Collins]

On Thu, 2009-03-19 at 09:19 +1000, Ian Clatworthy wrote:
> Robert Collins wrote:
> 
> > Let me ask again - what will it take to push the stuff that was broken
> > down into the tree object so that add wouldn't have broken at all - so
> > the bug isn't a UI bug.
> 
> I'm not sure at this point.
> 
> A large number of API deal in file-ids while views deal with paths so
> it's non-trivial to push it down everywhere. It is currently handled
> at the *path-processing layer* and that transparently works for 95+%
> of current and future commands, add being the notable exception.

Perhaps views should deal in fileids? That way they would not need
reconfiguring on 'mv' operations, nor suffer friction at the API layers.

> Either way, it's well down my list of priorities. Views are not blocking
> adoption while getting a brisbane-core format landed, EOL handling and
> logging multiple files/directories are.

Sure.

> > As it stands, it sounds like *every* UI will break with views, and thats
> > quite undesirable. How can I help us avoid that problem?
> 
> I think that's *dramatically* overstating the problem. Only add goes
> through the bit of broken code (tree_files_for_add()) and even add
> was working for simple cases when I tried to reproduce Mark's bug last
> night.
> 
> I think we're falling into the trap here of making perfect the enemy
> of better. This is a simple "one-line", *obvious* patch. It would have
> been found during review if everyone bothered to review the code in the
> 2 months I had it up for review!

Not all things are obvious during review, and I seem to recall that the
ui-code-has-to-change thing *was* raised during that period. I don't
think having a bug is something to panic about; I'm trying to address
what I perceive as a design issue, not a 'omg a user had a problem in a
beta format'.

>  We should get this patch approved and
> landed IMO and not block it on "redesign filtered views so we don't need
> to duplicate a small number of blackbox tests".

Uhm. AIUI 
tree = WorkingTree.open('.')
tree.stuff()

will not work correctly on a tree that has views.

Is this right or wrong?

If I've misunderstood, I'll be very relieved.

> I also appreciate the point about GUI tools needing changing to support
> views. They will anyway - e.g. a dialog for defining a view and a view
> selector widget.

Even in the common case of work-without-views-in-a-new-format-tree?

>  Adding a single line call to builtins.tree_files()
> is a small fraction of the code they need to duplicate out of the body
> of most UI commands. When the GUI guys take on this work, we can tweak
> things to assist them then.

I hate to be a stick in the mud, but:
pqm
all plugins
tarmac
$who-knows-how-many adhoc scripts

are all candidates for needing this hand holding. IF I have it wrong,
its a non issue and we can move on. If I have it right, then sure - we
don't need to block this patch (just don't multiply blackbox tests -
PLEASE) - but this may be an indication that views need more love to be
the tree format that chk testers see. Or something.

-Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20090319/03ea9b6d/attachment.pgp