Wonderful! That's a great way to get started. :-)
>I was looking at bug #449 which led me to
>fp_BlockLayout.cpp and fp_Run.cpp. I'm now trying to figure out exactly
>what the fp_BlockLayout and fp_Run are used for. There aren't any high
>level comments decribing the classes that I could find. Is their any
>documention decribing this part of the code? I get the idea that
>fp_DocLayout describes the formating of a complete document, and
>fp_BlockLayout.cpp and fp_Run.cpp describe the layout of smaller pieces
>of the document (is that right?). Any pointers to more information or
>even a short description of these classes would be a big help.
I can really empathize. The first time I dove through that code I got
pretty confused, too. However, once I figured it out, it was pretty easy to
find my way around. I tried to leave behind key tidbits in the relevant
header files, such as the following bit in fl_DocLayout.h:
/*
FL_DocLayout is a formatted representation of a specific PD_Document,
formatted for a specific GR_Graphics context.
A FL_DocLayout encapsulates two related hierarchies of objects.
The "logical" hierarchy corresponds to the logical structure of the
document, and consists of a list of fl_SectionLayout objects, which are
in turn composed of fl_BlockLayout objects.
The "physical" hierarchy, by contrast, encapsulates the subdivision
of physical space into objects of successively finer granularity. Thus,
a FL_DocLayout is also a list of Pages, each of which was constructed
during the process of formatting the document. In turn,
each fp_Page is a list of fp_Columns
Each fp_Column is a list of fp_Lines
Each fp_Line is a list of Runs.
Finally, each fp_Run contains some fragment of content from the original
document, usually text.
*/
I also renamed all the classes so the prefix would indicate whether they
were part of the logical (fl_) or physical (fp_) hierarchies. In accordance
with tree-wide naming conventions, capitalized prefixes indicate that the
class is exposed outside that module, and lowercase prefixes are
module-private.
>I haven't looked through much of the source yet, but I did notice that
>the files in abi/src/text/fmt/xp are hardly commented.
Having worked with Eric for many years, that's a clear indication of how he
works when he's in a hurry. :-)
You get a lot of code handling a very complex task which seems to be
internally consistent. However, you need to interpret the high-level design
from well-named functions and variables. The only comments you tend to find
are for local exceptions.
>I think that some
>high level comments describing the classes, interfaces, and data members
>would help people like me who are new to the code.
Absolutely.
>If I added comments
>to the code as I went through it, would you guys accept patches of my
>changes and incorporate them into your development tree?
Within reason, sure. In the past, I've had tech writers attempt to
translate large blocks of C into equally-verbose English prose, which gets
annoying fast. Anyone tempted to do so should let us know, and we can
definitely find better uses for their time. ;-)
However, roadmap-style comments like the one above would be greatly
appreciated. It's always nice when each header file includes an intro
comment explaining the scope and intent of that class, plus any design
concepts which can't be easily deduced within, say, 15 minutes of scanning
the underlying code.
As you peek through the rest of the source base, you'll notice that we've
been trying to make sure that there's a brief comment introducing each class
as we add (or maintain) them, but there are still a lot of key classes which
haven't been retrofitted yet. For example, the following formatter classes
all have an introductory comment:
FG_GraphicRaster
FG_Graphic
FL_DocLayout (and its associated fl_DocListener)
fl_Layout
fl_PartOfBlock
fp_Line
Unfortunately, you've found the densest set of unexplained code in the
program. None of the following classes are properly introduced:
fb_ColumnBreaker
fb_LineBreaker
fl_SectionLayout
fl_DocSectionLayout, fl_HdrFtrSectionLayout,
fl_HdrFtrShadow (and its associated fl_ShadowListener)
fl_BlockLayout
fp_Page
fp_Container
fp_Column, fp_HdrFtrContainer
fp_Run
fp_TextRun, fp_TabRun, fp_ForcedLineBreakRun,
fp_ForcedColumnBreakRun, fp_ForcedPageBreakRun, fp_ImageRun,
fp_FieldRun, fp_FmtMarkRun
FV_View
Once this last batch of classes gets taken care of, we'll be almost all the
way there.
>I did notice
>that the method and data member names are descriptive for the
>classesI looked at, which makes them pretty self documenting, but it's
>still hard to figure out what is going on when you just dive into the
>source for the first time.
Yep. Once you get over that initial hurdle, things get a *lot* better, but
that first step is a doozy. :-)
Paul