inline On Sat, Oct 22, 2011 at 12:04 AM, Chris Bowditch <bowditch_ch...@hotmail.com > 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.