In message <[EMAIL PROTECTED]> you wrote:
> Author: adamblokus
> Date: Sat Jul 26 06:54:36 2008
> New Revision: 4737
>
> [...]
> --- branches/adamblokus/netsurf/desktop/options.c (original)
> +++ branches/adamblokus/netsurf/desktop/options.c Sat Jul 26 06:54:36 2008
> @@ -136,6 +136,18 @@
> #else
> unsigned int option_min_reflow_period = 25; /* time in cs */
> #endif
> +/** top margin of exported page*/
> +int option_margin_top = DEFAULT_MARGIN_TOP;
> +/** bottom margin of exported page*/
> +int option_margin_bottom = DEFAULT_MARGIN_BOTTOM;
> +/** left margin of exported page*/
> +int option_margin_left = DEFAULT_MARGIN_LEFT;
> +/** right margin of exported page*/
> +int option_margin_right = DEFAULT_MARGIN_RIGHT;
> +/** scale of exported content*/
> +int option_export_scale = DEFAULT_EXPORT_SCALE;
> +/** directory where the exported file will be saved*/
> +char *option_export_directory = 0;
I think this can become "const char *" as you don't want other code
changing the string via this pointer (assuming this won't result in having
to cast the const away in other code, if it does feel free to leave this
as is).
Also I slightly prefer using NULL i.s.o. 0 for pointers. C standard wise it
boils down to the same but it is slightly more clear what this is all about.
> ==============================================================================
> --- branches/adamblokus/netsurf/desktop/print.c (original)
> +++ branches/adamblokus/netsurf/desktop/print.c Sat Jul 26 06:54:36 2008
> [...]
> + settings->scale = option_export_scale;
> + settings->scale /= 100;
I would turn this into one line.
> +
> + settings->margins[MARGINLEFT] = option_margin_left;
> + settings->margins[MARGINRIGHT] = option_margin_right;
> + settings->margins[MARGINTOP] = option_margin_top;
> + settings->margins[MARGINBOTTOM] = option_margin_bottom;
> +
> + settings->margins[MARGINTEXT] = DEFAULT_MARGIN_TEXT;
> +
> + strcpy(path, option_export_directory);
> + strcat(path, "/out.pdf");
This is potentionally not platform independent as not all platforms use
the '/' character as directory separator. I don't think it is a real
issue for platforms we're supporting right now (including RISC OS) so you
can leave it as is but adding a FIXME or Doxygen todo comment could avoid
grumble in the future.
> [...]
> ==============================================================================
> --- branches/adamblokus/netsurf/gtk/gtk_gui.c (original)
> +++ branches/adamblokus/netsurf/gtk/gtk_gui.c Sat Jul 26 06:54:36 2008
> @@ -273,7 +273,13 @@
> LOG(("Using '%s' as download directory", home));
> option_downloads_directory = home;
> }
> -
> +
> + if (!option_export_directory) {
> + char *home = getenv("HOME");
> + LOG(("Using '%s' as export directory", home));
When HOME is not defined, you're using NULL to print. With some C runtime
libraries you get away with this but don't count on it. Avoid this with
something like: home ? home : "<NULL>".
> [...]
> ==============================================================================
> --- branches/adamblokus/netsurf/pdf/font_haru.c (original)
> +++ branches/adamblokus/netsurf/pdf/font_haru.c Sat Jul 26 06:54:36 2008
> @@ -60,6 +60,8 @@
> };
>
>
> +extern float pdf_scale;
> +
'extern' in a C file rings my alarm bells...
> [...]
> ==============================================================================
> --- branches/adamblokus/netsurf/pdf/pdf_plotters.c (original)
> +++ branches/adamblokus/netsurf/pdf/pdf_plotters.c Sat Jul 26 06:54:36 2008
> +bool pdf_set_scale(float s)
> +{
> + pdf_scale = s;
> +}
Ahem, no 'return' ? Didn't the compiler warn you about this ?
I don't see a good reason to have a return value here. If you do, please
add some Doxygen comments explaining how to interpret that value (and make
sure you use that return value yourself in the code calling pdf_set_scale()).
>
> Modified: branches/adamblokus/netsurf/pdf/pdf_plotters.h
> URL:
> http://source.netsurf-browser.org/branches/adamblokus/netsurf/pdf/pdf_plotters.h?rev=4737&r1=4736&r2=4737&view=diff
> ==============================================================================
> --- branches/adamblokus/netsurf/pdf/pdf_plotters.h (original)
> +++ branches/adamblokus/netsurf/pdf/pdf_plotters.h Sat Jul 26 06:54:36 2008
> @@ -37,4 +37,7 @@
> /**Close pdf document and save changes to file*/
> void pdf_end(void);
>
> +float pdf_scale;
Tss, no 'extern' for variables in header ?!
Thanks,
John.
--
John Tytgat
[EMAIL PROTECTED]
