Re: [Plplot-devel] Planned conversion from /* ... */ to // form of comments this weekend

[Alan W. I. <ir...@be...>]

As usual this is taking longer than I had hoped, but I am making some
good progress.  I just (revision 11269) completed the second chunk of
source code (all include/*.h files except for qt.h).

On 2010-10-11 15:34-0700 Alan W. Irwin wrote:

> Here are the steps I did including all the checks.
>
> I.  A preliminary run of the script gave a runtime error because of
> one embedded comment, e.g.,
>
> function( /* this is the first argument*/ x )
>
> To my mind this is horrible comment style in any case (and I think
> everyone else agrees because there was only one instance).  I
> therefore edited the file to get rid of the embedded comment and the
> check for this issue (anything after the trailing */ on a line
> generates a runtime error) will continue to be done by the script.
>
> N.B. This means new instances of embedded comments will generate
> script errors, but I doubt our contributors will generate many (if
> any) embedded comments so I haven't worried about implementing a more
> graceful way than a run-time error to deal with this case.

Actually, in this chunk of C source code I ran into some embedded
comments that made sense.  So I now leave them unmolested (in the /*
... /* form) in single-line form (i.e., if they are not in the middle
of a block comment).

If embedded comments do occur by mistake in the middle of a block
comment or other comment style shows up which the script doesn't know
how to deal with, then scripts/convert_comment.py bails with a runtime
error.  Furthermore, I have modified scripts/style_source.sh to stop
with an overall error message when such runtime errors occur for
scripts/convert_comment.py.  This new logic worked fine for all files
in the latest chunck of source code except for include/plplotcanvas.h
which needed some hand editing (revison 11264) to change its
pathological comment style into something scripts/convert_comment.py
could understand.

>
> II. Just out of curiosity I checked src for any non-blank characters
> in front of // before any changes were applied.
>
> software@raven> grep '[^ ]\+.*//' src/*.[ch] |less
>
> As expected the small number of instances (mostly URL's inside comments)
> were legit.

software@raven> grep '[^ ]\+.*//' $(ls include/*|grep -v qt.h) |less

also showed small number of legit instances of //.

>
> III. While looking at the potential changes generated by the script
> (using "scripts/style_source.sh --diff -au |less") I checked inside
> the less command for the following two conditions:
>
> any trailing blanks;
> /. $
>
> any insertions that don't have / anywhere on the line (a valid insertion
> should always have // on the line so if there is no / anywhere, that
> is an error);
>
> /^[+][^/]+$
>
> These searches found nothing.

Ditto for second chunk.  I also searched in the context diff for any
line with a combination of "//" and "*" in it with the following two searches:

///.*\*

and

/\*.*//

A fair number of such patterns turned up, but they all looked like
mainstream commentary style (or trailing comment style in the second
case) which the python script appears to handle with absolutely no
problems.

>
> IV. I also eyeballed all the potential changes in the ~27000-line
> context diff to look for any strange patterns in the resulting //
> comments.  For various special reasons there were three instances of
> indentation errors for the comments that I spotted. (I have noted
> these issues and will fix by hand-editing later.) Other than these
> minor indentation issues I am completely satisfied with the style of
> the commentary now in src.
>
> convert_comment.py is fairly general and hopefully proof against most
> sorts of wacko styles of comments by our contributors.  However, there
> are no guarantees so the best results will be derived from now on by
> not straining this script too much.
>
> N.B. For example, if you stick with //-style comments from now on, the
> script is guaranteed to not attempt to change anything in the files.

For the second chunk, the context diff that I eyeballed was "only"
~9100 lines.  I didn't find any strange patterns in the resulting
comments (after the include/plplotcanvas.h case was handled as
discussed above).

>
> V. The python script is specifically designed not to add or delete any
> lines to source files. I checked the number of lines in the files
> before and after the changes were applied to confirm this was the
> case.

Same in this case.

>
> VI. After "scripts/style_source.sh --apply" was run, I found I had to
> run it a second time to take care of some minor realignment that
> uncrustify wanted to do because of the comment styling changes.  But
> subsequent runs showed no further changes were generated.

Same in this case.

>
> VII. I tested the new commentary style by running "cmake" and "make
> -j4 test_noninteractive" and "make -j4 test_interactive" in the build
> tree. No major build or run-time issues showed up as a result of those
> tests. Also, the visual results generated by "make -j4
> test_interactive" looked good (within the general limitation of how
> fast they are displayed).

Same in this case, but instead of doing those commands by hand, I ran

software@raven> scripts/comprehensive_test.sh \
--cmake_added_options -DPLD_pdf=ON  --build_command 'make -j4' \
--do_nondynamic no --do_static no --do_ctest no \
--do_test_install_tree no --do_test_traditional_install_tree no

just to test the extensive header changes in this second chunk as
well as to use that updated script in an interesting and useful way.

>
> N.B. Of course such test results are never complete so keep an eye out
> during this release cycle for any run-time bugs that may have been
> introduced by this change in commentary style.
>
> ToDo.  Go through the same steps for other chunks of our C code.

This second chunk is just a step in the right direction, and there
will be more to follow (by removing some of the current temporary
limits in scripts/style_source.sh) from e.g., lib, drivers, bindings,
and examples,

Alan
__________________________
Alan W. Irwin

Astronomical research affiliation with Department of Physics and Astronomy,
University of Victoria (astrowww.phys.uvic.ca).

Programming affiliations with the FreeEOS equation-of-state implementation
for stellar interiors (freeeos.sf.net); PLplot scientific plotting software
package (plplot.org); the libLASi project (unifont.org/lasi); the Loads of
Linux Links project (loll.sf.net); and the Linux Brochure Project
(lbproject.sf.net).
__________________________

Linux-powered Science
__________________________