[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Groff] PDFPIC macro
From: |
Keith Marshall |
Subject: |
Re: [Groff] PDFPIC macro |
Date: |
Mon, 29 Sep 2014 22:30:52 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.0 |
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).
--
Regards,
Keith.
psbb-process.patch
Description: Text Data
- Re: [Groff] PDFPIC macro, (continued)
- Re: [Groff] PDFPIC macro, Keith Marshall, 2014/09/21
- Re: [Groff] PDFPIC macro, Ulrich Lauther, 2014/09/21
- Re: [Groff] PDFPIC macro, Deri James, 2014/09/21
- Re: [Groff] PDFPIC macro, Keith Marshall, 2014/09/21
- Re: [Groff] PDFPIC macro, Keith Marshall, 2014/09/21
- Re: [Groff] PDFPIC macro, Deri James, 2014/09/21
- Re: [Groff] PDFPIC macro, Peter Schaffter, 2014/09/21
- Re: [Groff] PDFPIC macro, Keith Marshall, 2014/09/29
- Re: [Groff] PDFPIC macro, Steffen Nurpmeso, 2014/09/29
- Re: [Groff] PDFPIC macro, Werner LEMBERG, 2014/09/29
- Re: [Groff] PDFPIC macro,
Keith Marshall <=
- Re: [Groff] PDFPIC macro, Clarke Echols, 2014/09/29
- Re: [Groff] PDFPIC macro, Werner LEMBERG, 2014/09/30
- Re: [Groff] PDFPIC macro, Keith Marshall, 2014/09/30
- Re: [Groff] PDFPIC macro, Werner LEMBERG, 2014/09/30
- Re: [Groff] PDFPIC macro, Ralph Corderoy, 2014/09/30
- Re: [Groff] PDFPIC macro, Keith Marshall, 2014/09/18
- Re: [Groff] PDFPIC macro, Steffen Nurpmeso, 2014/09/19
- Re: [Groff] PDFPIC macro, Peter Schaffter, 2014/09/21
Re: [Groff] PDFPIC macro, Keith Marshall, 2014/09/17