Header comment formatting standards - draft 0.01
mbe...@glasscity.net
It seems appropriate to formalize some of these standards, so here goes.
Comments are appreciated.
Header Comment Formatting Standards
Version 0.01
March 31, 1998
Matthew Becker (mbe...@glasscity.net)
00. General
a. Do not use tabs - only spaces
b. Do not type any characters past column 80
c. Any BUGs or FIXMEs should be done as code statements with the
appropriate dbgmsg routine.
01. Function Name
a. FunctionName [space] SpecInfo
b. FunctionName starts in column 4 - or do we want it more centered?
c. SpecInfo is either [SpecFile.Number] or [Internal]
d. SpecFile name is in capital letters
e. SpecInfo enclosed in brackets [], not parenthesis ()
02. Function Description
a. Blank line before and after
b. Should it be a complete sentence?
c. For 16/32 and 32A/32W pairs, the one calling the other can just make
a reference to the other one, using the keyword 'See'
e.g. For Function Foo32A, the comment would be "See Foo32W."
03. Notes
a. Use keyword 'NOTES'
b. One blank line before and after
04. Return value
a. Use keyword 'RETURNS'
b. Each return 'type' get a separate line
c. Use keywords 'Success'/'Failure', which are separated by a colon, with
the description being first
d. What about using other keywords such as 'other'/'else'?
05. Function Parameters
a. Each parameter gets a separate line
b. What column to start on?
1. 4 (see example) - This lines it up with the code, then.
2. Column where actual function name begins - this might push the
comments too far to the right.
c. Parameters get in-line comment, beginning with [in], [out], or [in/out]
d. Comments are left-aligned on the first column available after the
right-most comma (see example)
e. Ending parameter parenthesis is put on same line as the beginning
brace with 1 space in-between.
Example:
/*****************************************************************************
**
* VirtualQuery [KERNEL32.554]
*
* Provides info about a range of pages in virtual address space.
*
* NOTES
* This is not exactly like the real code anymore.
*
* RETURNS
* Number of bytes returned in information buffer: Success
* 0: Failure
*/
DWORD WINAPI VirtualQuery(
LPCVOID addr, /* [in] Address of region */
LPMEMORY_BASIC_INFORMATION info, /* [in] Address of info buffer */
DWORD len /* [out] Size of buffer */
) {
/* Code goes here */
}
-----== Posted via Deja News, The Leader in Internet Discussion ==-----
http://www.dejanews.com/ Now offering spam-free web-based newsreading
Douglas Ridgway
>It seems appropriate to formalize some of these standards, so here goes.
>Comments are appreciated.
There are, I think, two connected issues:
1. Having quality comments in the code so that people reading the code
can understand what it's supposed to do.
2. Having quality documentation of the Wine API, which can be processed
into texinfo / nroff / html for the use of people writing winelib code.
A bit of formality aids the eye while reading the code, but also allows
c2man to generate nice looking docs.
There are still problems with the automatic documentation system, mostly
stemming from the fact that the function Foo() is implemented up to three
times: as Foo16(), Foo32A(), and Foo32W(). c2man generates three man
pages, all with the wrong names, and with the wrong types (HWND32 instead
of HWND, LPSTR or LPWSTR instead of LPTSTR, etc). Also, man pages for
purely internal functions are generated as well.
>00. General
> a. Do not use tabs - only spaces
> b. Do not type any characters past column 80
> c. Any BUGs or FIXMEs should be done as code statements with the
> appropriate dbgmsg routine.
I think that a BUGS section in the header is appropriate, if the problem
is known. If it's not in the header, it won't show up in the man page.
>01. Function Name
> a. FunctionName [space] SpecInfo
FunctionName should be spelled _precisely_ the same as the name of the
C implementation, including capitalization. Otherwise c2man will output
them both.
Also, I suggest that a short description appear here: eg
/******************************************************************
* PlayMetaFile32 (GDI32.265) Render metafile to device
*
This description appears in the NAME section of the man page, and
is what is searched by apropos. Good short descriptions will be useful
in the long run.
> b. FunctionName starts in column 4 - or do we want it more centered?
After processing, it doesn't matter, so this is a matter of source code
esthetics.
> c. SpecInfo is either [SpecFile.Number] or [Internal]
Internal functions are not part of the Wine API, and so they really
should not have man pages. That man pages are generated for many internal
functions is a bug in the documentation generation.
For internal functions then, the comments are still important, but
structure is less important. In general, the people reading the
comments will be looking directly at the code.
> d. SpecFile name is in capital letters
> e. SpecInfo enclosed in brackets [], not parenthesis ()
No objection, as long as it's not me who has to change thousands of
existing comments.
>02. Function Description
> a. Blank line before and after
> b. Should it be a complete sentence?
> c. For 16/32 and 32A/32W pairs, the one calling the other can just make
> a reference to the other one, using the keyword 'See'
> e.g. For Function Foo32A, the comment would be "See Foo32W."
If you are looking at the source code, it's usually pretty obvious where
you should go look. And as dicussed above, there shouldn't be separate
man pages for the various Foo's, as they are all (in principle) the same
function.
So which version of a function do we document? I suggest
that only the Foo32W() version of Wine API functions get the formal
header documentation. I think that it's a waste of effort to document them
all, as someone reading the code should have no trouble finding the
right place to look, and someone looking at a man page should (eventually)
only have one man page to look at.
What if there are differences between versions? Well, those differences
should be documented on the Foo32W() manpage. Eg,
/***********************************************
* Foo32W (FOOUSER.1) Do Foo
...
* BUGS
* UNICODE version unimplemented.
*/
{
return 0;
}
doug.
Morten Eriksen
In article <6fr1d0$o5n$1...@nnrp1.dejanews.com>,
mbe...@glasscity.net writes:
>
> It seems appropriate to formalize some of these standards, so here goes.
> Comments are appreciated.
Nice piece of work. I do have a number of suggestions, though, which I
hope could contribute to make the header comment standard useful not
only for documentation purposes, but also to speed up development and
help debugging.
The idea is that we should include keywords+information into the
headers which will help us generate reports in more detail about the
current development status of Wine - showing us what is done, what is
tested and what needs to be debugged, robustified and fixed.
We could then let a source processing system automatically generate
reports on how many (and which) functions have been implemented, which
functions have been robustified to handle invalid input parameters,
which and how many functions have been "proof-read" against the "real"
Windows OS'es API documentation, which functions have there been made
regression testing code for (testing the output against "real"
Windows, of course), which functions have been made thread-safe (if I
understand correctly I believe this is a huge and bloody important
task), etc etc. And what about "non-technical" metainformation about
the implementation, like AUTHOR (with email address) and so on?
So in my opinion we need quite a few more keywords (too many would be
bad, of course, we should try to keep it on a minimum while still
including all the important information).
A source processing system could generate nice HTML pages with this
info, listing the functions (grouped by DLL belonging) along with
color coding showing their implementation status (green - fully
implemented and well tested, yellow - not tested/not thread-safe,
orange - partly implemented (FIXMEs and known unhandled cases of input
parameters still present), red - no implementation at all so
far). Along with these reports, there could be URL-pointers to
"official" API documentation and Wine-generated documentation, the
actual source code, and so on.
I believe a system like this would be extremely nice for a number of
reasons:
* it would make it easier for new-comers to quickly get an overview of
the status of Wine and find out where they want to spend their
efforts (this is way too hard at the present time, IMHO).
* "seasoned developers" would get a better overview of the parts of
Wine they haven't been working on, but which perhaps still influence
pieces of code they have written.
* the same "seasoned developers" would probably find it rewarding to
watch a "percentage to go" "countdown clock" tick down towards zero
before a certain DLL or even the whole Wine system is complete (I know
I would find this an incitement and great inspiration, at least). :-)
* it would be easier to trace possible problems with specific programs
(check the output of -debugmsg +relay against all these function's
respective implementation status, start at the top and fix all
problems you see).
> 00. General
> a. Do not use tabs - only spaces
> b. Do not type any characters past column 80
> c. Any BUGs or FIXMEs should be done as code statements with the
> appropriate dbgmsg routine.
A few practical observation here:
* your format specification is too rigid regarding whitespace,
limitations on field lengths, needed keywords, etc. It will be a mess
to try to enforce this. From the beginning on, we'll need allow *all*
keywords and information to be missing - to get a head start and not
break anything.
* the best route to meet the goal of a "header parser and
documentation generation" tool would probably be to grab one of the
existing tools which are under open-source licenses (perhaps one of
these: KDOC <http://www.ph.unimelb.edu.au/~ssk/kde/kdoc/>, cocoon
<http://www-users.cs.umn.edu/~kotula/cocoon/cocoon.htm>, DOC++
<http://www.zib.de/Visual/software/doc++/index.html> or doxygen
<http://www.stack.nl/~dimitri/doxygen/>), and hack it until it does
what we need. This would solve the previous point I made about too
rigid definition aswell, as I assume these systems have pretty
"relaxed" rules for extracting information.
Including the documentation/report generation into the wine make
process would be a good thing also, IMO, as all developers would then
have an instant up-to-date copy on their harddisk upon download and
compile of the latest wine snapshot (no need to curse slow internet
connections or web-sites which are infrequently updated).
> 04. Return value
[SNIP]
> 05. Function Parameters
[SNIP]
I like these, they are a must for making API documentation independent
of Crimosoft.
And in case anybody is wondering why I'm not contributing to this
myself already: it's the usual crap - too much bloody "real" work, too
little bloody time, and too many bleeding projects with my involvement
already.
I do submit the occational patch for bug fixes and new (simple)
function implementations, though. Wine is one of the projects with the
highest "fun factor" I've ever encountered.
Regards,
Morten Eriksen
Claus Fischer
rid...@taiga.gmcl.com quoted mbe...@glasscity.net writing:
> a. Do not use tabs - only spaces
> b. Do not type any characters past column 80
For emacs-addicts it might be useful to have such a line at the start
of the files:
/* -*-Mode: C; indent-tabs-mode: nil; fill-column: 79; -*- */
although actually enabling the auto-fill-mode will definitely be a matter of
user preferrence.
Claus
cfis...@td2cad.intel.com
not speaking for Intel
David Lee Lambert
On 2 Apr 1998, Claus Fischer wrote:
> rid...@taiga.gmcl.com quoted mbe...@glasscity.net writing:
> > a. Do not use tabs - only spaces
> > b. Do not type any characters past column 80
>
> For emacs-addicts it might be useful to have such a line at the start
> of the files:
>
> /* -*-Mode: C; indent-tabs-mode: nil; fill-column: 79; -*- */
>
> although actually enabling the auto-fill-mode will definitely be a matter of
> user preferrence.
My Emacs window has 71 character positions. I find it annoying when stuff
is longer than that, because little pieces get pushed onto the next line.
I have auto-fill-mode set for 70 columns.
--
David Lee Lambert MHm 16x20 Hack Programmer and Student
Admin of lmert.dyn.ml.org http://lmert.dyn.ml.org
My web pages are at http://web.egr.msu.edu/~lamber45
E-mail: m/lamber45\100(egr|pilot)\.msu\.edu/
The world would be a nicer place if everyone knew karate, rode bicycles and
used Unix. Unfortunately, some people have promiscous unprotected sex, drive
drunk and use Win95.
Dimitrie O. Paun
On 2 Apr 1998, Douglas Ridgway wrote:
[...]
> There are still problems with the automatic documentation system, mostly
> stemming from the fact that the function Foo() is implemented up to three
> times: as Foo16(), Foo32A(), and Foo32W(). c2man generates three man
> pages, all with the wrong names, and with the wrong types (HWND32 instead
> of HWND, LPSTR or LPWSTR instead of LPTSTR, etc). Also, man pages for
> purely internal functions are generated as well.
All these problems (and much more) could be solved if we use our own
documentation generator.
[...]
> I think that a BUGS section in the header is appropriate, if the problem
> is known. If it's not in the header, it won't show up in the man page.
That's right -- a BUGS section would be nice. I don't think that even a
custom tool could replace such a section (by, say, recovering the FIXMEs
from the body).
> >01. Function Name
> > a. FunctionName [space] SpecInfo
>
> FunctionName should be spelled _precisely_ the same as the name of the
> C implementation, including capitalization. Otherwise c2man will output
> them both.
An easy check for wdoc :)
> Also, I suggest that a short description appear here: eg
>
> /******************************************************************
> * PlayMetaFile32 (GDI32.265) Render metafile to device
> *
>
> This description appears in the NAME section of the man page, and
> is what is searched by apropos. Good short descriptions will be useful
> in the long run.
I don't like it very much because the function names are long and this
leaves little space for the descriptions. I would put the description
_below_ the function, in one sentence, followed by a blank line, like this:
/******************************************************************
* PlayMetaFile32 (GDI32.265)
* Render metafile to device
*
wdoc would recover it simply by etting all text after the function name
up to the first blank line.
> > c. SpecInfo is either [SpecFile.Number] or [Internal]
>
> Internal functions are not part of the Wine API, and so they really
> should not have man pages. That man pages are generated for many internal
> functions is a bug in the documentation generation.
Yeap, but wdoc would look at the stuff in [...] and decide what to do with
it. Very important. I think the [...] is _very_ important to generate good
docs.
> > d. SpecFile name is in capital letters
> > e. SpecInfo enclosed in brackets [], not parenthesis ()
>
> No objection, as long as it's not me who has to change thousands of
> existing comments.
We can change this easy with a script. Not a problem.
Dimi
--
Dimitrie O. Paun <di...@cs.toronto.edu> |
Graduate Student, Computer Science | Friends don't let friends boot NT!
University of Toronto |
User on David Lee Lambert's computer
Morten Eriksen wrote:
> The idea is that we should include keywords+information into the
> headers which will help us generate reports in more detail about the
> current development status of Wine - showing us what is done, what is
> tested and what needs to be debugged, robustified and fixed.
>
> We could then let a source processing system automatically generate
> reports on how many (and which) functions have been implemented, which
> functions have been robustified to handle invalid input parameters,
> which and how many functions have been "proof-read" against the "real"
> Windows OS'es API documentation, which functions have there been made
> regression testing code for (testing the output against "real"
> Windows, of course), which functions have been made thread-safe (if I
> understand correctly I believe this is a huge and bloody important
> task), etc etc. And what about "non-technical" metainformation about
> the implementation, like AUTHOR (with email address) and so on?
Before I found out that the apps list on winehq.com wasn't done with
SQL, I thought we could create a database there to keep track of
four things:
1. All the tests that have been done with Wine;
2. All the programs we've tested;
3. All the authors and testers, with contact information;
and
4. All the Win32 functions, what programs use them, how they are
supposed to work, whether and how well they actually work in each
implementation (Win 3.x, Win95, WinNT, Win32s, Wine, Willows, WABI,
FreeWin98, VNC, etc.), and who wrote or maintains the Wine version.
I guess my idea was to have up to three Wine-specific fields in the
function-table -- 16, 32A, 32W -- which would be automatically
generated from the source code.
> So in my opinion we need quite a few more keywords (too many would be
> bad, of course, we should try to keep it on a minimum while still
> including all the important information).
It may be better to keep some info in a seperate file. In particular,
perhaps we should generate one manual of how Win32 *should* work that
applies to both Wine and TWIN.
> A source processing system could generate nice HTML pages with this
> info, listing the functions (grouped by DLL belonging) along with
> color coding showing their implementation status (green - fully
> implemented and well tested, yellow - not tested/not thread-safe,
> orange - partly implemented (FIXMEs and known unhandled cases of input
> parameters still present), red - no implementation at all so
> far). Along with these reports, there could be URL-pointers to
> "official" API documentation and Wine-generated documentation, the
> actual source code, and so on.
Or, if we had a seperate database on wineHQ.com (or some other place),
it would be trivial to export that database as flat-file text or HTML.
>
> I believe a system like this would be extremely nice for a number of
> reasons:
>
> * it would make it easier for new-comers to quickly get an overview of
> the status of Wine and find out where they want to spend their
> efforts (this is way too hard at the present time, IMHO).
>
> * "seasoned developers" would get a better overview of the parts of
> Wine they haven't been working on, but which perhaps still influence
> pieces of code they have written.
>
> * the same "seasoned developers" would probably find it rewarding to
> watch a "percentage to go" "countdown clock" tick down towards zero
> before a certain DLL or even the whole Wine system is complete (I know
> I would find this an incitement and great inspiration, at least). :-)
>
> * it would be easier to trace possible problems with specific programs
> (check the output of -debugmsg +relay against all these function's
> respective implementation status, start at the top and fix all
> problems you see).
all good points
> Including the documentation/report generation into the wine make
> process would be a good thing also, IMO, as all developers would then
> have an instant up-to-date copy on their harddisk upon download and
> compile of the latest wine snapshot (no need to curse slow internet
> connections or web-sites which are infrequently updated).
>
> > 04. Return value
> [SNIP]
> > 05. Function Parameters
> [SNIP]
>
> I like these, they are a must for making API documentation independent
> of Crimosoft.
>
We do need this.
David Lee Lambert
On 4 Apr 1998, Dimitrie O. Paun wrote:
> On Fri, 3 Apr 1998, User on David Lee Lambert's computer wrote:
>
> [...]
> > > So in my opinion we need quite a few more keywords (too many would be
> > > bad, of course, we should try to keep it on a minimum while still
> > > including all the important information).
> >
> > It may be better to keep some info in a seperate file. In particular,
> > perhaps we should generate one manual of how Win32 *should* work that
> > applies to both Wine and TWIN.
>
> I would vote against it. The whole purpose of this thread is to find a way
> to _avoid_ putting information in separate files. Experience shows that,
> unless strictly enforced, this strategy results (really fast) in
> out-of-sync files.
But the point is that what a function should do (as opposed to what it
does do) is inherently out of sync with the development effort.
--
David Lee Lambert MHm 16x20 Hack Programmer and Student
Admin of lmert.dyn.ml.org http://lmert.dyn.ml.org
My web pages are at http://web.egr.msu.edu/~lamber45
E-mail: m/lamber45\100(egr|pilot)\.msu\.edu/
The world would be a nicer place if everyone knew karate, rode bicycles and
Dimitrie O. Paun
On Fri, 3 Apr 1998, User on David Lee Lambert's computer wrote:
[...]
> > So in my opinion we need quite a few more keywords (too many would be
> > bad, of course, we should try to keep it on a minimum while still
> > including all the important information).
>
> It may be better to keep some info in a seperate file. In particular,
> perhaps we should generate one manual of how Win32 *should* work that
> applies to both Wine and TWIN.
I would vote against it. The whole purpose of this thread is to find a way
to _avoid_ putting information in separate files. Experience shows that,
unless strictly enforced, this strategy results (really fast) in
out-of-sync files.
Dimi
Dimitrie O. Paun
On Fri, 3 Apr 1998, David Lee Lambert wrote:
> On 4 Apr 1998, Dimitrie O. Paun wrote:
>
> > On Fri, 3 Apr 1998, User on David Lee Lambert's computer wrote:
> >
> > [...]
> > > > So in my opinion we need quite a few more keywords (too many would be
> > > > bad, of course, we should try to keep it on a minimum while still
> > > > including all the important information).
> > >
> > > It may be better to keep some info in a seperate file. In particular,
> > > perhaps we should generate one manual of how Win32 *should* work that
> > > applies to both Wine and TWIN.
> >
> > I would vote against it. The whole purpose of this thread is to find a way
> > to _avoid_ putting information in separate files. Experience shows that,
> > unless strictly enforced, this strategy results (really fast) in
> > out-of-sync files.
>
> But the point is that what a function should do (as opposed to what it
> does do) is inherently out of sync with the development effort.
Yes, but this does not justify in any way a methodology that would enlarge
the gap. You are right, by the very nature of Wine we will always be
behind but what we are trying to do is to minimize this as much as
possible. One way to do this is by automating some of the tasks that can
be automated. And one of the areas where we can clearly do it, is in the
documentation part. Wine has become very big and is harder and harder for
someone to start hacking on Wine. The person needs deep understanding of
the Windows API, and a very good knowledge of the Wine internals. Well,
now it is a very difficult task to learn about Wine. The proposal we are
discussing would help in this area.