[Okular-devel] [PATCH] Fix locateFonts

[Albert Astals Cid]

A Dimarts, 27 d'octubre de 2009, Igor Stirbu va escriure:
> Hello,
> 
> I've attached a slightly corrected version for previous patch.

Some comments:
 * I know it's not on the file style, but can you please add m_ in front of 
the name of member variables you are adding, makes it much easier to track if 
they are function or member variables (if you change the other member 
variables good too)
 * Make the enum start by capital letter, that file doesn't follow style 
either, but classes/enums etc should better start by capital letter
 * Wondering if locateState = LOCATE_FONT_FIRST; should go to 
fontPool::locateFonts() instead in the constructor
 * You don't seem to be updating makePK and locateTFMonly correctly, makePK is 
always false when in
  if (!areFontsLocated())
    locateFonts(true, false);
was true and i'd say locateTFMonly in LOCATE_FONT_PK should be true not false
 * Also in the original code there is a loop
 do {
    vffound = false;
    locateFonts(false, false, &vffound);
  } while(vffound);
that i don't see where you do that

Don't take my comments let you down, we are happy to have you contributing :-)

Albert

> 
> Thanks,
> Igor
>