inline
On Sat, Oct 22, 2011 at 12:04 AM, Chris Bowditch <[email protected]
> wrote:
>
> Since Thunderhead also needs this feature we are willing to invest some
> time into it too. Currently my team are telling me it would take 9 person
> months to port this code into our branch of FOP, partly because of some
> merge conflicts, but also partly because we are not comfortable with some
> aspects of the code as it has already been pointed out. The estimate would
> include the time to refactor long files into small ones, deal with the
> variable names and restructuring the code.
>
I would advise against this, since it would it is functionally unnecessary
and since it will make future merges more difficult. I will be making
additional changes as more features in this area are added.
I don't see what refactoring long files into small ones buys you. However,
if you make a reasoned argument for factoring specific long files (i.e., why
such factoring improves architecture, modularity, etc), rather than simply
say "all long files must be refactored", then I will seriously discuss doing
so post merge.
> I appreciate your commitment to add comments to short identifiers
> declarations, so at least it will be easier for the team to translate the
> short variables to longer equivalents. Just so we are clear on what you
> propose, do you mean this:
>
> int gi = 0; // Glyph Index
>
Yes. Note that I already do this in most cases, such as:
private static void resolveExplicit ( int[] wca, int defaultLevel, int[] ea
) {
int[] es = new int [ MAX_LEVELS ]; /* embeddings stack */
int ei = 0; /* embeddings stack index */
int ec = defaultLevel; /* current embedding level
*/
for ( int i = 0, n = wca.length; i < n; i++ ) {
int bc = wca [ i ]; /* bidi class of current
char */
int el; /* embedding level to assign
to current char */
switch ( bc ) {
case LRE: // start left-to-right
embedding
case RLE: // start right-to-left
embedding
case LRO: // start left-to-right
override
case RLO: // start right-to-left
override
{
int en; /* new embedding level */
if ( ( bc == RLE ) || ( bc == RLO ) ) {
en = ( ( ec & ~OVERRIDE ) + 1 ) | 1;
} else {
en = ( ( ec & ~OVERRIDE ) + 2 ) & ~1;
}
if ( en < ( MAX_LEVELS + 1 ) ) {
es [ ei++ ] = ec;
if ( ( bc == LRO ) || ( bc == RLO ) ) {
ec = en | OVERRIDE;
} else {
ec = en & ~OVERRIDE;
}
} else {
// max levels exceeded, so don't change level or
override
}
el = ec;
break;
}
...
What I'm agreeing to do in the relative near future (after merge, before new
release) is to add such comments to those places where I have not already
done so, which are probably a minority of such cases.
G.
