I prefer an approach that allows print to SVG and PNG down the road...  YMMV

I need this for Epiphany. Currently gecko can only produce postscript, which I have to send to the printer with gtk_print_job_send.
So I need both a print-to-postscript option *and* a way to disable print-to-pdf (since the pdf backend cannot convert the postscript stream to pdf).

would adding a GENERATE_PDF capability be a good way to hide the 
"print to pdf" in the printer list ?

A GENERATE_PDF capability sounds like a good idea.

I would prefer to make this print-to-file backend with an option of the format, and setting the capabilities only makes some formats (pdf) unavailable, not the whole print-to-file concept.

Well, if none of the supported formats can be generated, we should obviously
hide the print-to-file backend from the ui...

2006-06-12  Matthias Clasen  <mclasen@redhat.com>

	* gtk/gtkprintjob.h: Add a GTK_PRINT_CAPABILITY_GENERATE_PDF
	capability.
	
	* gtk/gtkprintunixdialog.c: Use it here to hide "Print to PDF" 
	from the printer list.  (#339592, Christian Persch)

Needs some more work to actually get the printer's ability to accept
PDF/PS. Right now, its hardcoded to look for _("Print to PDF").

Created attachment 67442 [details] [review]
first patch, for comments

This patch makes the pdf backend in a print-to-file backend (I haven't renamed/moved the file yet since that would make the diff unreadable).

There are a couple of problems remaining:
- I'd like the Format combo to show in the main page besides the filename input instead of on a page of its own, but haven't found a way yet
- if the app can only provide ps or only pdf, the printer shouldn't offer to switch; I haven't found out how to do that

If you give the format printeroption the same special name "gtk-main-dialog-custom-input", it'll show up in that spot.
But that seems slightly wrong, I don't think the extension point
should be picked by option name (which seems to imply that 
there can only be one option per extension point). Instead,
they should be picked by group, I think, and there should
be one special group for the widgets that end up on the
first

For the second issue, gtkprintunixdialog.c probably needs to
translate the GENERATE_PDF (and a new GENERATE_PS) into settings
so that they are available to the backend printer code when
it adds the options to the print dialog.

Ok, I have committed the change to add a GtkPrintSettings argument to
the create_cairo_surface printer method.

I think I'll split the option placement problem into a separate bug and work on that after this one is resolved, otherwise the patch becomes too involved/big.

So I don't understand what the GENERATE_PDF flag is supposed to mean....

gtk_print_unix_dialog_set_manual_capabilities says:
 * This lets you specify the printing capabilities your application
 * supports. For instance, if you can handle scaling the output then
 * you pass #GTK_PRINT_CAPABILITY_SCALE. If you don't pass that, then
 * the dialog will only let you select the scale if the printing
 * system automatically handles scaling.

But I don't want to generate PDF/PS output, I want to deliver the *input* to the printer in a stream in that format... so I think this flag should actually be reversed? gtkprintunixdialog does:
     if ((priv->manual_capabilities & GTK_PRINT_CAPABILITY_GENERATE_PDF) == 0)
	result = gtk_printer_accepts_ps (printer);
when instead it should do
      if (priv->manual_capabilities & GTK_PRINT_CAPABILITY_GENERATE_PDF)
	result = gtk_printer_accepts_pdf (printer);
      else if (priv->manual_capabilities & GTK_PRINT_CAPABILITY_GENERATE_PS)
	result = gtk_printer_accepts_ps (printer);

Does that make sense, or am I just not understanding this correctly?

there is two sides here: the format(s) accepted by the printer, and
what the application can provide. 

The accepts_ps and accepts_pdf functions are meant to cover the first (what can the printer accept) and GENERATE_PDF/GENERATE_PS are meant to indicate what the
application can create.

If you are using the highlevel api, then the app can always create both (since 
cairo has ps and pdf surfaces). The print-to-file printer can always accept both
(since it just writes it to a file)

I believe your rewriting of the matching logic is roughly equivalent to 
what is already there.

I have now cleaned up the handling of option groups, and it should be easy
to add the format selection to the main tab.

Created attachment 67525 [details]
this is easilty possible now

Created attachment 67551 [details] [review]
updated patch

I have now added the GENERATE_PS capability and capabilities are now
passed to printer_get_options, so that the file backend can decide
whether or not to show the file format option based on the GENERATE_X
capabilities.

Christian, if you update your patch, can you please split it in a
series of independent patches, that makes it easier to review, a la:

1) add export-format and export-uri print settings
2) make pdf backend support postscript output
3) rename pdf backend to file backend

Created attachment 67630 [details] [review]
step 1

Add EXPORT_URI and EXPORT_FILE_FORMAT settings, and fix gtkprinteroptionwidget to put an URI into the setting (and fix the encoding issue there).

Next up: rename to "file" backend.

Created attachment 67635 [details] [review]
step 2

Move from pdf/ to file/ and renamed to GtkPrintBackendPdf to GtkPrintBackendFile (with patch from step 1 applied, of course).

Comments on "step 1":

- I don't think switching from a filename to an uri will buy us much
  (although I don't have strong feelings about it).

- I think the printer-specific export- options should not be defined
  in gtkprintsettings.h

- I wonder if the naming export- confuses print-to-file and the export
  action even more. Maybe we should make the export action use these
  settings ? Need to discuss with alex

- I think the FILESAVE code in gtkprinteroptionwidget.c should not
  hardcode "Print to file", but use the display_text of the option.

(In reply to comment #19)
> Comments on "step 1":
> 
> - I don't think switching from a filename to an uri will buy us much
>   (although I don't have strong feelings about it).
How I see programs using this is like this: the user inputs a non-local URI (so the setting really has to be an URI). Program saves the setting, changes to that to temp file, prints, then uploads the temp file to the real location with gnome-vfs.
And the encoding fix is really necessary.
And this also ensures that the setting value is UTF-8, so I can store it in a keyfile.

> - I think the printer-specific export- options should not be defined
>   in gtkprintsettings.h
They should be defined *somewhere*, otherwise people won't know what option to use unless they read the source. Is there any other appropriate header?

> - I wonder if the naming export- confuses print-to-file and the export
>   action even more. Maybe we should make the export action use these
>   settings ? Need to discuss with alex
I don't care about the name... it was "pdf-filename", both parts wrong :)
"print-to-uri" ? "print-destination-uri" ? "destination-uri" ?

> - I think the FILESAVE code in gtkprinteroptionwidget.c should not
>   hardcode "Print to file", but use the display_text of the option.

I'll fix that.

Created attachment 67647 [details] [review]
step 1 updated

Use the display name as filechoose title.

Although there's some inconsistency here, since this doesn't select the filename to print to, just the directory... but that's for another bug.

> > - I think the printer-specific export- options should not be defined
> >   in gtkprintsettings.h
> They should be defined *somewhere*, otherwise people won't know what option to
> use unless they read the source. Is there any other appropriate header?

I guess my point is that people really should not have to know about driver-specific options. These should solely be used for communication 
between the backend and the option widgets.

There is one legitimate use of that setting by the program itself: when you manage your printing not with the gtkprintoperation but just use gtkprintjob, you can optimise the case of print-to-file by just printing directly to that file instead of a temp file which you then gtk_print_job_send.

here is the highlights of my quick discussion with alex:

<alex> +  file = g_locale_from_utf8 (gtk_entry_get_text (GTK_ENTRY (priv->entry)),
<alex> +        -1, NULL, NULL, NULL);
<alex> Surely the filename shouldn't be in locale encoding
<mclasen> right, it needs to be in filename encoding


- We really don't see the point in going through the uri; 
  lets keep it a filename for now. 

- There is potential for confusion between the export- settings and  
  the EXPORT action. Better names to use: output-filename and output-format

(In reply to comment #24)
> here is the highlights of my quick discussion with alex:
> 
> <alex> +  file = g_locale_from_utf8 (gtk_entry_get_text (GTK_ENTRY
> (priv->entry)),
> <alex> +        -1, NULL, NULL, NULL);
> <alex> Surely the filename shouldn't be in locale encoding
> <mclasen> right, it needs to be in filename encoding
Damn, I used the wrong function! I'll fix that.

> - We really don't see the point in going through the uri; 
>   lets keep it a filename for now. 
The point is to keep it open to future support for non-local files (bug 344522). If we keep it filename now we need to change it later and programs need to support both filenames and uris.

> - There is potential for confusion between the export- settings and  
>   the EXPORT action. Better names to use: output-filename and output-format
Ok, I'll change that. 

(In reply to comment #25)
> > - We really don't see the point in going through the uri; 
> >   lets keep it a filename for now. 
> The point is to keep it open to future support for non-local files (bug
> 344522). If we keep it filename now we need to change it later and programs
> need to support both filenames and uris.

Also, filenames have arbitrary encodings while URIs are UTF-8. This simplifies storing/restoring the GktPrintSettings and makes them unambiguous.

Created attachment 67721 [details] [review]
step 1 update 2

Fix to use g_filename_from_utf8 and use OUTPUT instead of EXPORT.

ok, looks good. 
I'm willing to accept the argument that uris have a defined encoding.
Can you commit this  ?

2006-06-20  Christian Persch  <chpe@cvs.gnome.org>

        * gtk/gtkprinteroptionwidget.c: (filesave_changed_cb),
        (alternative_set), (construct_widgets): Convert input to filename
        encoding and construct an URI from that.
        * gtk/gtkprintsettings.h: Add OUTPUT_FILE_FORMAT and OUTPUT_URI keys.
        * modules/printbackends/pdf/gtkprintbackendpdf.c: Use those defines.
        Step 1 from bug #339592.

Created attachment 67727 [details] [review]
step 2 updated

Looks good, go ahead

2006-06-20  Christian Persch  <chpe@cvs.gnome.org>

        * configure.in
        * gtk/Makefile.am
        * modules/printbackends/Makefile.am
        A modules/printbackends/file/.cvsignore
        A modules/printbackends/file/Makefile.am
        A modules/printbackends/file/gtkprintbackendfile.c
        A modules/printbackends/file/gtkprintbackendfile.h
        R modules/printbackends/pdf/*:
        Rename "pdf" backend to "file" backend. Step 2 from bug #339592.

Are you going to handle step 3 too, or should I go ahead and do that ?

Created attachment 67735 [details] [review]
step 3

Add print-to-PS option.

I'm hesitating between ALTERNATIVE and PICKONE type for the extra option... both look wrong since they expand vertically since the filesave option has 2 rows... still we can fix that up later.

Why are you showing the file format option if there is only one choice ?
I had imagined to only show it if there is more than one choice. Of course,
it would be nice to change the name back to  "Print to PS/PDF" in that case...

Also, why do you treat the can-generate-neither-ps-nor-pdf specially ?
I think we should just expect applications to set the capabilities
correctly. 

Finally, I think it would be a nice touch to change the default output filename
according to the supported formats: output.ps/output.pdf according to
the initially selected format (shouldn't that be initialized from the settings,
btw ? )

(In reply to comment #35)
> Why are you showing the file format option if there is only one choice ?
> I had imagined to only show it if there is more than one choice. Of course,
> it would be nice to change the name back to  "Print to PS/PDF" in that case...
I'll do that.

> Also, why do you treat the can-generate-neither-ps-nor-pdf specially ?
> I think we should just expect applications to set the capabilities
> correctly. 
The (neither GENERATE_PS nor GENERATE_PDF are set) case is not "I'm not going to print anything" case, it's the "I'm going to print to the cairo surface" case, in which we can generate any supported format, PS *and* PDF.

> Finally, I think it would be a nice touch to change the default output filename
> according to the supported formats: output.ps/output.pdf according to
> the initially selected format (shouldn't that be initialized from the settings,
> btw ? )
I'll fix that.

(In reply to comment #36)
> (In reply to comment #35)
> > Why are you showing the file format option if there is only one choice ?
> > I had imagined to only show it if there is more than one choice. Of course,
> > it would be nice to change the name back to  "Print to PS/PDF" in that case...
> I'll do that.

I don't think you can, with the current GtkPrinter api, since name is a 
construct-only property.

> > Also, why do you treat the can-generate-neither-ps-nor-pdf specially ?
> > I think we should just expect applications to set the capabilities
> > correctly. 
> The (neither GENERATE_PS nor GENERATE_PDF are set) case is not "I'm not going
> to print anything" case, it's the "I'm going to print to the cairo surface"
> case, in which we can generate any supported format, PS *and* PDF.

Hmm, at least the high-level api sets both GENERATE_PS and GENERATE_PDF,
so it wouldn't need this special case.

(In reply to comment #37)
> > > it would be nice to change the name back to  "Print to PS/PDF" in that case...
> > I'll do that.
> 
> I don't think you can, with the current GtkPrinter api, since name is a 
> construct-only property.
Yes :(

> > > Also, why do you treat the can-generate-neither-ps-nor-pdf specially ?
> > > I think we should just expect applications to set the capabilities
> > > correctly. 
> > The (neither GENERATE_PS nor GENERATE_PDF are set) case is not "I'm not going
> > to print anything" case, it's the "I'm going to print to the cairo surface"
> > case, in which we can generate any supported format, PS *and* PDF.
> 
> Hmm, at least the high-level api sets both GENERATE_PS and GENERATE_PDF,
> so it wouldn't need this special case.

If we take GENERATE_PS/PDF to say "allow the user to print to PS/PDF", then if the application hardcodes these flags, and we add print-to-SVG later (which the print-to-cairo-surface path can easily support with no changes to the application) those apps wouldn't offer this choice even though they do support that... 

(In reply to comment #38)
> > Hmm, at least the high-level api sets both GENERATE_PS and GENERATE_PDF,
> > so it wouldn't need this special case.
> 
> If we take GENERATE_PS/PDF to say "allow the user to print to PS/PDF", then if
> the application hardcodes these flags, and we add print-to-SVG later (which the
> print-to-cairo-surface path can easily support with no changes to the
> application) those apps wouldn't offer this choice even though they do support
> that... 

Interesting argument. Although I would expect most print-to-cairo users
to go through the hilevel api, in which case they would get print-to-svg, since
we would update GtkPrintOperation to advertise that capability...

Anyway, if you want to support this NONE==ALL special case, it needs to 
be documented in some way. The docs for GtkPrintCapabilities are in
one of the template files, I forget which.

Created attachment 67805 [details] [review]
step 3 updated

I'm  sorry, I actually went ahead already and committed a tweaked  version of your earlier patch...

Really pressed for time today, sorry


Anyway,  I at least took the doc additions from your updated patch.