Jean-Marc Lasgouttes wrote:
Abdelrazak Younes <[EMAIL PROTECTED]> writes:Here is the patch. I think it is easy to understand. So objection? Index: Bidi.cpp =================================================================== --- Bidi.cpp (revision 19485) +++ Bidi.cpp (working copy) @@ -227,14 +227,14 @@ * cursor gets stuck. */ return cur.bottom().paragraph().isRightToLeftPar( - cur.bv().buffer()->params()); + cur.bv().buffer().params()); }At the first one, I have an objection already :) This is typically the kind of change the should be done afterwards, since this obscures the meaning of the patch and is probably half of the changes.
As usual you all focus on the cleanliness of the patch, not of the actual code. Look at the branch!
docstring text = bformat(_("The document %1$s could not be saved.\n\n" - "Do you want to rename the document and " - "try again?"), file); + "Do you want to rename the document and try again?"), file);This change does not belong here and personally I think it makes things worse (as several hunks below).
I've noticed and reverted those. This is the typical work one have to do when merging a branch.
I'll stop there, not because I think I proved a point, but because it is late and the bv::buffer change makes it painful to get to the essence of the patch.
But why don't you review the branch instead? The incremental commit are much more easier to grasp. Why do you insist on reviewing the final patch?
There is one point that intrigues me: what is the special handling for child documents?
Look at loadChildDocuments(). It's all in the commit logs. Abdel.
