Re: [Groff] PDFPIC macro

[Clarke Echols]

I agree with Keith.  I've often over the years found my plentiful
comments very helpful in going back and reviewing or modifying code.

I even comment a lot of my HTML/XHTML work as well as CSS files so
I can readily skim what I've created and refresh my memory before
attempting useful modifications or updates.

I learned troff by taking the man macros from AT&T Unix (which had
no comments), and proceeded to make them a new file that was fully
commented, while no breaking the code.

Every line had to be commented, explaining what it does or how it
works.  That forced a level of understanding that has been helpful
in numerous other pursuits since.

It's a good habit to form for any creator.  It helps one become a
better writer as well, by forcing structured thinking and well
structured sentences and text.

CE

On 09/29/2014 03:30 PM, Keith Marshall wrote:
> On 29/09/14 21:20, Werner LEMBERG wrote:
> > > I've refactored the appropriate code, in src/roff/troff/input.cpp,
> > > with a view to accommodating this; see patch attached.  While I've
> > > not yet progressed any implementation for PDF handling, I have
> > > indicated the point at which it should be invoked.  Okay to commit,
> > > thus far?
> >
> > This is very nice, thanks!  For my taste, the comments are a bit
> > excessive, but I guess this is probably only me who thinks so :-)
>
> I've always favoured a verbose commenting style, since I learned to
> write FORTRAN-66, way back in the early 1970s.  Today, with my failing
> 60+ year old memory, I really appreciate the value of this, when I come
> back to code 6 months or so after I wrote it, and find that I cannot
> remember how it was supposed to work :-)
>
> > Some comments, mainly regarding formatting:
> >
> > +             while ((context = bounding_box_args()) == NULL
> > +             &&     get_line(DSC_LINE_MAX_ENFORCE) > 0)
> > +               ;
> >
> > Please say
> >
> >               while ((context = bounding_box_args()) == NULL
> >                      && get_line(DSC_LINE_MAX_ENFORCE) > 0)
> >                 ;
>
> Okay.
>
> > +      while (*context == '\x20' || *context == '\t')
> > +               context++;
>
> Hmm.  I don't know how that crept in ...
>
> > Please say
> >
> >        while (*context == '\x20' || *context == '\t')
> >           context++;
>
> This is how it *looked* in my working file copy, but there's one tab in
> amongst a string of spaces there -- I must have somehow managed to
> override vim's normal tab insertion on that line.  The extra '+' flag,
> in the patch file, pushed it over by an extra tab stop, and I missed it
> when I cast my eye over the patch.  It should be fixed, in the updated
> patch attached.
>
> > +      status = (context_args("(atend)", context) == NULL)
> > +       ? llx = lly = urx = ury = PSBB_RANGE_IS_BAD
> > +       : PSBB_RANGE_AT_END;
> >
> > Please say
> >
> >        status = (context_args("(atend)", context) == NULL)
> >                  ? llx = lly = urx = ury = PSBB_RANGE_IS_BAD
> >                  : PSBB_RANGE_AT_END;
>
> Okay.
>
> > +  register size_t len = strlen(tag);
> >
> > Given that recent versions of clang emit warnings if it encounters the
> > `register' keyword, and given that this keyword has no effect for
> > about 20 years, please don't use it:
>
> Done, but are you sure about the lack of effect?  (I seem to recall
> having observed a benefit, quite recently, in assembly code generated by
> GCC-4.8.2).