Hello Werner, > While I fully agree that groff's source code files should have much > more comments, I'm not really happy with the layout you provide in > your patch. James Clark's code has a certain compactness which I > would like to retain
Personal preferences about style obviously differ. Thank you for having adjusted it to your preferences. > I also prefer having no variable names in declarations. Then you need to use 'arg1', 'arg2', 'arg3' to refer to the arguments... > . Please provide patches with option -u which makes it easier for me > to read them. This is also a matter of personal taste, and also a function of the size of the hunks. Fortunately there are tools for converting such patches to the other format, at http://www.haible.de/bruno/gnu/diffconvert-1.3.tar.gz > . Use `XXX' instead of 'XXX' and "XXX" in comments. Well, I used "XXX" only when talking about strings, such as ",c". And for `XXX' I apologize: in all systems I use nowadays this looks asymmetric and therefore ugly, therefore normally I don't even contemplate to use this style. > . Use //, not /* ... */ where applicable. As you like. > . Don't use `character' if you really mean `glyph'. It is quite a > hard job to get a clean separation between these two things within > the groff sources due to history. The distinction wasn't clear in the beginning. Now it's becoming clearer. I'll watch this when doing the 'int' -> 'glyph' or 'glyph_t' type patch. > . There appears to be a fundamental misunderstanding of glyph boxes. > Similar to TeX, a glyph box is always a rectangle, even for > slanted fonts. Consequently, I've replaced all occurrences of > `parallelogram' with `rectangle'. > > . I rewrote the explanation of `skew'. Thanks! These are things that cannot be clear to a beginner. I used the term "parallelogram" to care about slanted glyphs. For example, while the backslash glyph in a font may have a width of 7 pixels, in a slanted version of the same font the width - defined as the horizontal dimension of the bounding box - will be only 5 pixels or so. Since the get_width() function doesn't take into account the slant, it will return 7 pixels. Therefore I thought the rectangle model is not the right one to describe the get_width() function. Here's an additional comment patch, to make get_height() and get_depth() clearer to those who already know what "ascent" and "descent" is. (See e.g. http://en.wikipedia.org/wiki/Typeface) Bruno --- src/include/font.h 26 Jan 2006 15:15:00 -0000 1.8 +++ src/include/font.h 26 Jan 2006 16:36:01 -0000 @@ -61,15 +61,17 @@ int get_height(int, int); // A rectangle represents the shape of the // glyph with the given index (arg1) at the given // point size (arg2). Return the distance between - // the base line and the top of this rectangle. If - // the top is above the base line, this value is - // positive. + // the base line and the top of this rectangle. + // This is often also called the 'ascent' of the + // glyph. If the top is above the base line, this + // value is positive. int get_depth(int, int); // A rectangle represents the shape of the // glyph with the given index (arg1) at the given // point size (arg2). Return the distance between // the base line and the bottom of this rectangle. - // If the bottom is below the base line, this value - // is positive. + // This is often also called the 'descent' of the + // glyph. If the bottom is below the base line, + // this value is positive. int get_space_width(int); // Return the normal width of a space at the // given point size. int get_character_type(int); // Return a bit mask describing the shape of _______________________________________________ Groff mailing list Groff@gnu.org http://lists.gnu.org/mailman/listinfo/groff