[MERGE][1.6] lookup rules using get_special_file API
John Arbash Meinel
john at arbash-meinel.com
Tue Jul 29 15:44:13 BST 2008
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Ian Clatworthy wrote:
| During the review of the rules-based preferences patch,
| John and I discussed the idea of accessing the .bzrrules
| file via a custom API which could provide better performance.
|
| During my testing of the content filtering branch, it turns
| out that using a custom API is actually a requirement in
| order to fix an infinite recursion bug that occurs. This
| patch introduces that API and uses it to find the .bzrrules
| file.
|
| The API has been designed so that it can be used for looking
| up .bzrignore and other special files down the track. This
| patch doesn't do that yet. I'm happy to add that now if the
| reviewer feels that I should. OTOH, that change can come later
| while I'd like this fix to rules in 1.6 if time permits.
|
| Ian C.
|
So a few comments:
1) In test_tree_implementations you add the file with the id "xyz-id" so
it isn't 100% clear that it isn't mapping via adding "-id". It is a
small thing, but it might be clearer that the file-id is actually a
random one.
2) I'm a bit concerned that the base 'Tree' implementation requires the
file to be versioned, but the one in WorkingTree does not.
For something like 'get_special_file' it feels like you should be doing
a lookup check, to make sure that .bzrrules is versioned, even if it
isn't committed yet.
Or did you explicitly want to support unversioned special files?
3) So I would like to see a workingtree_implementations test that
defines the behavior of non-versioned special files. I'll leave it to
you to decide whether they get returned or not.
4) Oh, and I'd rather get_special... returned either Lines, Chunks, or
Text. Mostly because then we don't have to worry about the caller
explicitly calling '.close()' on the returned object. I don't think most
callers are going to think about it, and not try/finally:foo.close()ing
your files causes problems on windows.
(I would really like to be switching apis to be 'chunks' as "lines" and
[fulltext] are both proper chunks and eliminate the need to be casting
between types all the time, but for this api, plain "get_file_bytes"
"get_file_text" or "get_file_lines()" are fine.)
I believe on Tree we would do either _text or _lines.
BB:tweak
John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iEYEARECAAYFAkiPLL0ACgkQJdeBCYSNAAPO6ACgn6xxYu7xzybPyMAe5sDwntJz
ZzAAoImjTjQUeeBL3bFuS62xtXlREaIH
=Eyzp
-----END PGP SIGNATURE-----
More information about the bazaar
mailing list