Re: [Groff] comments

[Bruno Haible]

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