Merge lp:~schwieni/inkscape/inkscape into lp:~inkscape.dev/inkscape/trunk

[schwieni]

Updates bug #669748

Add a command-line switch to specify PDF Export level.

Additional command line option added to export to different PDF-versions supported by cairo (currently only "PDF 1.4" and "PDF 1.5"). The user must provide the exact string found in the PDF-export dialog of Inkscape.
This feature was previously only accessible via the GUI of Inkscape.
This option is useful for people opting for PDF-a conformance in their PDFs. Moreover, the Extension and Parameter classes are extended by setters for enum parameters (used in combo boxes).

[lp:~schwieni/inkscape/inkscape]

12306. By schwieni on 2013-04-29

edited inkscape.pod for the --export-pdf-version command line option

12307. By schwieni on 2013-04-30

Fixed the false warning for the --pdf-export-version
Added functions to test if a given string is listed in the ComboBox.

12308. By schwieni on 2013-04-30

The command line option for --pdf-export-version is now unquouted e.g. --pdf-export-version=1.4
Default pdf export version now PDF 1.4

[Vinipsmaker (vinipsmaker)]

Why does ParamComboBox::contains receive doc and node arguments if they aren't used?

[schwieni (schwieni)]

You are right. I just followed the interface definition of the ::get
method, which doesn't use those arguments neither (but possibly for
future use/ usage in the past, I don't know). However, since I have no
personal preference to it, this can be changed. Should I change it?
Am 02.06.2013 23:05, schrieb Vinipsmaker:
> Why does ParamComboBox::contains receive doc and node arguments if they aren't used?

[Vinipsmaker (vinipsmaker)]

> You are right. I just followed the interface definition of the ::get
method, which doesn't use those arguments neither (but possibly for
future use/ usage in the past, I don't know). However, since I have no
personal preference to it, this can be changed. Should I change it?

Never mind. I just looked in the Inkscape source code and all code is written exactly that way. You are maintaing the consistent and that is good.

A separate patch (unrelated with yours) should be provided by somebody else to refactor these classes. I bet it's this code was inherited from the ancient C source code.

Your patch looks fine. Just one note: Shouldn't Extension::get_param_enum_contains be const?

[schwieni (schwieni)]

Extension::get_param_enum_contains(...) might be made const. Again, i just followed the other existing functions, which also might be made const. However, I did not test that and just assume that they can be made const.
cf. e.g. int get_param_int(...) etc.

[Vinipsmaker (vinipsmaker)]

Branch merged.

Patched code fixed in r12350:
http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/12350/src/main.cpp (line 1673).

[schwieni (schwieni)]

I don't understand exactly why line 1673 in main.cpp was changed in this
patch!? The original version is meant to set the default PDF version to
1.4 in *any* case, even if no explicit (and possibly wrong) pdf-version
string was provided. So, by adding the test if(sp_export_pdf_version &&
...) is IMHO not as intended. There was the wish to have a *predictable*
behavior in PDF-export by command line (cf. Bug #1110549:
https://bugs.launchpad.net/inkscape/+bug/1110549). However, in the
modified code, if no "--pdf-export-version ..." command line is given,
the last option set in the GUI is used (which is not predictable from
CLI). Furthermore, help and man-pages must be changed too with this
modification.

Therefore, I suggest to change line 1673 back as it was.

Am 05.06.2013 23:15, schrieb Vinipsmaker:
> Branch merged.
>
> Patched code fixed in r12350:
> http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/12350/src/main.cpp (line 1673).

[Vinipsmaker (vinipsmaker)]

> I don't understand exactly why line 1673 in main.cpp was changed in this
> patch!? The original version is meant to set the default PDF version to
> 1.4 in *any* case, even if no explicit (and possibly wrong) pdf-version
> string was provided. So, by adding the test if(sp_export_pdf_version &&
> ...) is IMHO not as intended. There was the wish to have a *predictable*
> behavior in PDF-export by command line (cf. Bug #1110549:
> https://bugs.launchpad.net/inkscape/+bug/1110549). However, in the
> modified code, if no "--pdf-export-version ..." command line is given,
> the last option set in the GUI is used (which is not predictable from
> CLI). Furthermore, help and man-pages must be changed too with this
> modification.
>
> Therefore, I suggest to change line 1673 back as it was.

I'm sorry. I used the wrong variable.

The first patch makes Inkscape crash when someone try to export a PS file (the affected function is used to export (E)PS and PDF files). r12352 have the right correction:

http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/12352

I'll try to discuss the patch more next time.

[schwieni (schwieni)]

In this case I would enclose line 1652 to 1676 from main.cpp with
"if(sp_export_pdf) { ... }" rather than only the last lines

Am 06.06.2013 10:52, schrieb Vinipsmaker:
> The first patch makes Inkscape crash when someone try to export a PS file (the affected function is used to export (E)PS and PDF files). r12352 have the right correction:
>
> http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/12352
>
> I'll try to discuss the patch more next time.

[Vinipsmaker (vinipsmaker)]

> In this case I would enclose line 1652 to 1676 from main.cpp with
> "if(sp_export_pdf) { ... }" rather than only the last lines

Provide another patch please. I can only code Inkscape when I get home (my netbook is too slow to Inkscape code base).

[Vinipsmaker (vinipsmaker)]

> In this case I would enclose line 1652 to 1676 from main.cpp with
> "if(sp_export_pdf) { ... }" rather than only the last lines

I'm home now. Looked at the source code and used a third option.

If the user try to export the pdf and ps versions at one run, the sp_export_pdf detection would fail. A better detection approach is to check the mime argument.

Change done in r12354.

[schwieni (schwieni)]

I'm sorry where can I find r12354. Is it possible that you forgot to
push the changes?

> I'm home now. Looked at the source code and used a third option.
>
> If the user try to export the pdf and ps versions at one run, the sp_export_pdf detection would fail. A better detection approach is to check the mime argument.
>
> Change done in r12354.

[Vinipsmaker (vinipsmaker)]

> I'm sorry where can I find r12354. Is it possible that you forgot to
> push the changes?

My mistake. I forgot to push. r12359:
http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/12359

[schwieni (schwieni)]

OK, perfect. Thank you very much for transferring my modifications into
inkscape.dev! I assume the feature will be there in the next inkscape
version (0.49), so I can delete my own branch on launchpad, right?
> My mistake. I forgot to push. r12359:

[Vinipsmaker (vinipsmaker)]

> OK, perfect. Thank you very much for transferring my modifications into
> inkscape.dev! I assume the feature will be there in the next inkscape
> version (0.49), so I can delete my own branch on launchpad, right?

Yes, your changes were integrated in Inkscape source code and it's safe to delete your own branch on launchpad.

I'm sorry for my poor social skills. Thanks for the patience.

=== modified file 'inkscape.pod'
--- inkscape.pod	2012-10-09 11:36:06 +0000
+++ inkscape.pod	2013-04-30 18:52:28 +0000
@@ -32,6 +32,7 @@
     -P, --export-ps=FILENAME
     -E, --export-eps=FILENAME
     -A, --export-pdf=FILENAME
+        --export-pdf-version=VERSION-STRING
         --export-latex
 
     -T, --export-text-to-path
@@ -268,6 +269,13 @@
 specify --export-id to export a single object (all other are hidden); in that case 
 export area is that object's bounding box, but can be set to page by --export-area-page.
 
+=item B<--export-pdf-version>=I<PDF-VERSION>
+
+Select the PDF version of the exported PDF file. This option basically
+exposes the PDF version selector found in the PDF-export dialog of the
+GUI. You must provide one of the versions from that combo-box,
+e.g. "1.4". The default pdf export version is "1.4".
+
 =item B<--export-latex>
 
 (for PS, EPS, and PDF export)
 
=== modified file 'src/extension/extension.cpp'
--- src/extension/extension.cpp	2013-03-25 17:42:31 +0000
+++ src/extension/extension.cpp	2013-04-30 18:52:28 +0000
@@ -427,6 +427,21 @@
     return param->get_enum(doc, node);
 }
 
+/**
+ * This is useful to find out, if a given string \c value is selectable in a ComboBox named \cname.
+ * 
+ * @param name The name of the enum parameter to get.
+ * @param doc The document to look in for document specific parameters.
+ * @param node The node to look in for a specific parameter.
+ * @return true if value exists, false if not
+ */
+bool
+Extension::get_param_enum_contains(gchar const * name, gchar const * value, SPDocument * doc, Inkscape::XML::Node * node)
+{
+    Parameter * param = get_param(name);
+    return param->get_enum_contains(value, doc, node);
+}
+
 gchar const *
 Extension::get_param_optiongroup( gchar const * name, SPDocument const * doc, Inkscape::XML::Node const * node)
 {
@@ -597,6 +612,13 @@
     return param->set_optiongroup(value, doc, node);
 }
 
+gchar const *
+Extension::set_param_enum(gchar const * name, gchar const * value, SPDocument * doc, Inkscape::XML::Node * node)
+{
+    Parameter * param = get_param(name);
+    return param->set_enum(value, doc, node);
+}
+
 
 /**
     \return   The passed in value
 
=== modified file 'src/extension/extension.h'
--- src/extension/extension.h	2013-03-14 01:33:10 +0000
+++ src/extension/extension.h	2013-04-30 18:52:28 +0000
@@ -244,6 +244,11 @@
                                         SPDocument const *   doc = 0,
                                         Inkscape::XML::Node const * node = 0);
 
+    bool             get_param_enum_contains(gchar const * name,
+                                             gchar const * value,
+                                             SPDocument  * doc = 0x0,
+                                             Inkscape::XML::Node * node = 0x0);
+
     bool             set_param_bool   (const gchar * name,
                                        bool          value,
                                        SPDocument *   doc = NULL,
@@ -269,6 +274,11 @@
                                         SPDocument * doc = 0,
                                         Inkscape::XML::Node * node = 0);
 
+    gchar const *    set_param_enum   (gchar const * name,
+                                       gchar const * value,
+                                       SPDocument * doc = 0x0,
+                                       Inkscape::XML::Node * node = 0x0);
+
     guint32          set_param_color  (const gchar * name,
                                        guint32 color,
                                        SPDocument *   doc = NULL,
 
=== modified file 'src/extension/param/enum.cpp'
--- src/extension/param/enum.cpp	2012-11-26 10:33:19 +0000
+++ src/extension/param/enum.cpp	2013-04-30 18:52:28 +0000
@@ -173,6 +173,24 @@
     return _value;
 }
 
+/**
+ * function to test if \c guitext is selectable
+ */
+bool ParamComboBox::contains(const gchar * guitext, SPDocument const * /*doc*/, Inkscape::XML::Node const * /*node*/) const
+{
+    if (guitext == NULL) {
+        return false; /* Can't have NULL string */
+    }
+
+    for (GSList * list = choices; list != NULL; list = g_slist_next(list)) {
+        enumentry * entr = reinterpret_cast<enumentry *>(list->data);
+        if ( !entr->guitext.compare(guitext) )
+            return true;
+    }
+    // if we did not find the guitext in this ParamComboBox:
+    return false;
+}
+
 void
 ParamComboBox::changed (void) {
 
 
=== modified file 'src/extension/param/enum.h'
--- src/extension/param/enum.h	2012-02-29 01:16:51 +0000
+++ src/extension/param/enum.h	2013-04-30 18:52:28 +0000
@@ -51,6 +51,11 @@
 
     const gchar * set (const gchar * in, SPDocument * doc, Inkscape::XML::Node * node);
 
+    /**
+     * @returns true if guitext is part of this enum
+     */
+    bool contains(const gchar * guitext, SPDocument const * /*doc*/, Inkscape::XML::Node const * /*node*/) const;
+
     void changed (void);
 }; /* class ParamComboBox */
 
 
=== modified file 'src/extension/param/parameter.cpp'
--- src/extension/param/parameter.cpp	2012-02-29 01:16:51 +0000
+++ src/extension/param/parameter.cpp	2013-04-30 18:52:28 +0000
@@ -183,6 +183,15 @@
     return param->get(doc, node);
 }
 
+bool Parameter::get_enum_contains(gchar const * value, SPDocument const *doc, Inkscape::XML::Node const *node) const
+{
+    ParamComboBox const *param = dynamic_cast<ParamComboBox const *>(this);
+    if (!param) {
+        throw Extension::param_not_enum_param();
+    }
+    return param->contains(value, doc, node);
+}
+
 gchar const *Parameter::get_optiongroup(SPDocument const *doc, Inkscape::XML::Node const * node) const
 {
     ParamRadioButton const *param = dynamic_cast<ParamRadioButton const *>(this);
@@ -247,6 +256,15 @@
     return param->set(in, doc, node);
 }
 
+gchar const *Parameter::set_enum( gchar const * in, SPDocument * doc, Inkscape::XML::Node * node )
+{
+    ParamComboBox *param = dynamic_cast<ParamComboBox *>(this);
+    if (!param) {
+        throw Extension::param_not_enum_param();
+    }
+    return param->set(in, doc, node);
+}
+
 
 /** Wrapper to cast to the object and use it's function. */
 guint32
 
=== modified file 'src/extension/param/parameter.h'
--- src/extension/param/parameter.h	2012-02-29 01:16:51 +0000
+++ src/extension/param/parameter.h	2013-04-30 18:52:28 +0000
@@ -88,9 +88,11 @@
     gchar const *get_enum(SPDocument const *doc, Inkscape::XML::Node const *node) const;
 
     /** Wrapper to cast to the object and use it's function. */
+    bool get_enum_contains(gchar const * value, SPDocument const *doc, Inkscape::XML::Node const *node) const;
+
+    /** Wrapper to cast to the object and use it's function. */
     gchar const *get_optiongroup(SPDocument const * doc, Inkscape::XML::Node const *node) const;
 
-
     /** Wrapper to cast to the object and use it's function. */
     bool set_bool(bool in, SPDocument * doc, Inkscape::XML::Node * node);
 
@@ -101,6 +103,8 @@
 
     gchar const *set_optiongroup(gchar const *in, SPDocument * doc, Inkscape::XML::Node *node);
 
+    gchar const *set_enum(gchar const * in, SPDocument * doc, Inkscape::XML::Node *node);
+
     gchar const *set_string(gchar const * in, SPDocument * doc, Inkscape::XML::Node * node);
 
     guint32 set_color(guint32 in, SPDocument * doc, Inkscape::XML::Node * node);
 
=== modified file 'src/main.cpp'
--- src/main.cpp	2013-04-06 14:17:29 +0000
+++ src/main.cpp	2013-04-30 18:52:28 +0000
@@ -148,6 +148,7 @@
     SP_ARG_EXPORT_PS,
     SP_ARG_EXPORT_EPS,
     SP_ARG_EXPORT_PDF,
+    SP_ARG_EXPORT_PDF_VERSION,
     SP_ARG_EXPORT_LATEX,
 #ifdef WIN32
     SP_ARG_EXPORT_EMF,
@@ -201,6 +202,7 @@
 static gchar *sp_export_ps = NULL;
 static gchar *sp_export_eps = NULL;
 static gchar *sp_export_pdf = NULL;
+static gchar *sp_export_pdf_version = NULL;
 #ifdef WIN32
 static gchar *sp_export_emf = NULL;
 #endif //WIN32
@@ -245,6 +247,7 @@
         sp_export_ps = NULL;
         sp_export_eps = NULL;
         sp_export_pdf = NULL;
+        sp_export_pdf_version = NULL;
 #ifdef WIN32
         sp_export_emf = NULL;
 #endif //WIN32
@@ -386,6 +389,12 @@
      N_("Export document to a PDF file"),
      N_("FILENAME")},
 
+    {"export-pdf-version", 0,
+     POPT_ARG_STRING, &sp_export_pdf_version, SP_ARG_EXPORT_PDF_VERSION,
+     // TRANSLATORS: "--export-pdf-version" is an Inkscape command line option; see "inkscape --help"
+     N_("Export PDF to given version. (hint: make sure to input the exact string found in the PDF export dialog, e.g. \"PDF 1.4\" which is PDF-a conformant)"),
+     N_("PDF_VERSION")},
+
     {"export-latex", 0,
      POPT_ARG_NONE, &sp_export_latex, SP_ARG_EXPORT_LATEX,
      N_("Export PDF/PS/EPS without text. Besides the PDF/PS/EPS, a LaTeX file is exported, putting the text on top of the PDF/PS/EPS file. Include the result in LaTeX like: \\input{latexfile.tex}"),
@@ -1638,6 +1647,33 @@
     }
     (*i)->set_param_float("bleed", margin);
 
+    // handle --export-pdf-version
+    bool set_export_pdf_version_fail=true;
+    const gchar *pdfver_param_name="PDFversion";
+    if(sp_export_pdf_version) {
+        // combine "PDF " and the given command line
+        std::string version_gui_string=std::string("PDF ")+sp_export_pdf_version;
+        try{
+            // first, check if the given pdf version is selectable in the ComboBox
+            if((*i)->get_param_enum_contains("PDFversion", version_gui_string.c_str())) {
+                (*i)->set_param_enum(pdfver_param_name, version_gui_string.c_str());
+                set_export_pdf_version_fail=false;
+            } else {
+                g_warning("Desired PDF export version \"%s\" not supported! Hint: input one of the versions found in the pdf export dialog e.g. \"1.4\".",
+                          sp_export_pdf_version);
+            }
+        } catch (...) {
+            // can be thrown along the way:
+            // throw Extension::param_not_exist();
+            // throw Extension::param_not_enum_param();
+            g_warning("Parameter or Enum \"%s\" might not exist",pdfver_param_name);
+        }
+    }
+    // set default pdf export version to 1.4, also if something went wrong
+    if(set_export_pdf_version_fail) {
+        (*i)->set_param_enum(pdfver_param_name, "PDF 1.4");
+    }
+
     //check if specified directory exists
     if (!Inkscape::IO::file_directory_exists(uri)) {
         g_warning("File path \"%s\" includes directory that doesn't exist.\n", uri);