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

Proposed by schwieni on 2013-04-26
Status: Merged
Merged at revision: 12348
Proposed branch: lp:~schwieni/inkscape/inkscape
Merge into: lp:~inkscape.dev/inkscape/trunk
Diff against target: 275 lines (+122/-1)
8 files modified
inkscape.pod (+8/-0)
src/extension/extension.cpp (+22/-0)
src/extension/extension.h (+10/-0)
src/extension/param/enum.cpp (+18/-0)
src/extension/param/enum.h (+5/-0)
src/extension/param/parameter.cpp (+18/-0)
src/extension/param/parameter.h (+5/-1)
src/main.cpp (+36/-0)
To merge this branch: bzr merge lp:~schwieni/inkscape/inkscape
Reviewer Review Type Date Requested Status
Inkscape Developers 2013-04-26 Pending
Review via email: mp+161225@code.launchpad.net

Commit message

Updates bug #669748

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

Description of the change

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).

To post a comment you must log in.
lp:~schwieni/inkscape/inkscape updated on 2013-04-30
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) wrote :

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

schwieni (schwieni) wrote :

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) wrote :

> 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) wrote :

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) wrote :

Branch merged.

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

schwieni (schwieni) wrote :

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) wrote :

> 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) wrote :

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) wrote :

> 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) wrote :

> 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) wrote :

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) wrote :

> 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) wrote :

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) wrote :

> 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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'inkscape.pod'
2--- inkscape.pod 2012-10-09 11:36:06 +0000
3+++ inkscape.pod 2013-04-30 18:52:28 +0000
4@@ -32,6 +32,7 @@
5 -P, --export-ps=FILENAME
6 -E, --export-eps=FILENAME
7 -A, --export-pdf=FILENAME
8+ --export-pdf-version=VERSION-STRING
9 --export-latex
10
11 -T, --export-text-to-path
12@@ -268,6 +269,13 @@
13 specify --export-id to export a single object (all other are hidden); in that case
14 export area is that object's bounding box, but can be set to page by --export-area-page.
15
16+=item B<--export-pdf-version>=I<PDF-VERSION>
17+
18+Select the PDF version of the exported PDF file. This option basically
19+exposes the PDF version selector found in the PDF-export dialog of the
20+GUI. You must provide one of the versions from that combo-box,
21+e.g. "1.4". The default pdf export version is "1.4".
22+
23 =item B<--export-latex>
24
25 (for PS, EPS, and PDF export)
26
27=== modified file 'src/extension/extension.cpp'
28--- src/extension/extension.cpp 2013-03-25 17:42:31 +0000
29+++ src/extension/extension.cpp 2013-04-30 18:52:28 +0000
30@@ -427,6 +427,21 @@
31 return param->get_enum(doc, node);
32 }
33
34+/**
35+ * This is useful to find out, if a given string \c value is selectable in a ComboBox named \cname.
36+ *
37+ * @param name The name of the enum parameter to get.
38+ * @param doc The document to look in for document specific parameters.
39+ * @param node The node to look in for a specific parameter.
40+ * @return true if value exists, false if not
41+ */
42+bool
43+Extension::get_param_enum_contains(gchar const * name, gchar const * value, SPDocument * doc, Inkscape::XML::Node * node)
44+{
45+ Parameter * param = get_param(name);
46+ return param->get_enum_contains(value, doc, node);
47+}
48+
49 gchar const *
50 Extension::get_param_optiongroup( gchar const * name, SPDocument const * doc, Inkscape::XML::Node const * node)
51 {
52@@ -597,6 +612,13 @@
53 return param->set_optiongroup(value, doc, node);
54 }
55
56+gchar const *
57+Extension::set_param_enum(gchar const * name, gchar const * value, SPDocument * doc, Inkscape::XML::Node * node)
58+{
59+ Parameter * param = get_param(name);
60+ return param->set_enum(value, doc, node);
61+}
62+
63
64 /**
65 \return The passed in value
66
67=== modified file 'src/extension/extension.h'
68--- src/extension/extension.h 2013-03-14 01:33:10 +0000
69+++ src/extension/extension.h 2013-04-30 18:52:28 +0000
70@@ -244,6 +244,11 @@
71 SPDocument const * doc = 0,
72 Inkscape::XML::Node const * node = 0);
73
74+ bool get_param_enum_contains(gchar const * name,
75+ gchar const * value,
76+ SPDocument * doc = 0x0,
77+ Inkscape::XML::Node * node = 0x0);
78+
79 bool set_param_bool (const gchar * name,
80 bool value,
81 SPDocument * doc = NULL,
82@@ -269,6 +274,11 @@
83 SPDocument * doc = 0,
84 Inkscape::XML::Node * node = 0);
85
86+ gchar const * set_param_enum (gchar const * name,
87+ gchar const * value,
88+ SPDocument * doc = 0x0,
89+ Inkscape::XML::Node * node = 0x0);
90+
91 guint32 set_param_color (const gchar * name,
92 guint32 color,
93 SPDocument * doc = NULL,
94
95=== modified file 'src/extension/param/enum.cpp'
96--- src/extension/param/enum.cpp 2012-11-26 10:33:19 +0000
97+++ src/extension/param/enum.cpp 2013-04-30 18:52:28 +0000
98@@ -173,6 +173,24 @@
99 return _value;
100 }
101
102+/**
103+ * function to test if \c guitext is selectable
104+ */
105+bool ParamComboBox::contains(const gchar * guitext, SPDocument const * /*doc*/, Inkscape::XML::Node const * /*node*/) const
106+{
107+ if (guitext == NULL) {
108+ return false; /* Can't have NULL string */
109+ }
110+
111+ for (GSList * list = choices; list != NULL; list = g_slist_next(list)) {
112+ enumentry * entr = reinterpret_cast<enumentry *>(list->data);
113+ if ( !entr->guitext.compare(guitext) )
114+ return true;
115+ }
116+ // if we did not find the guitext in this ParamComboBox:
117+ return false;
118+}
119+
120 void
121 ParamComboBox::changed (void) {
122
123
124=== modified file 'src/extension/param/enum.h'
125--- src/extension/param/enum.h 2012-02-29 01:16:51 +0000
126+++ src/extension/param/enum.h 2013-04-30 18:52:28 +0000
127@@ -51,6 +51,11 @@
128
129 const gchar * set (const gchar * in, SPDocument * doc, Inkscape::XML::Node * node);
130
131+ /**
132+ * @returns true if guitext is part of this enum
133+ */
134+ bool contains(const gchar * guitext, SPDocument const * /*doc*/, Inkscape::XML::Node const * /*node*/) const;
135+
136 void changed (void);
137 }; /* class ParamComboBox */
138
139
140=== modified file 'src/extension/param/parameter.cpp'
141--- src/extension/param/parameter.cpp 2012-02-29 01:16:51 +0000
142+++ src/extension/param/parameter.cpp 2013-04-30 18:52:28 +0000
143@@ -183,6 +183,15 @@
144 return param->get(doc, node);
145 }
146
147+bool Parameter::get_enum_contains(gchar const * value, SPDocument const *doc, Inkscape::XML::Node const *node) const
148+{
149+ ParamComboBox const *param = dynamic_cast<ParamComboBox const *>(this);
150+ if (!param) {
151+ throw Extension::param_not_enum_param();
152+ }
153+ return param->contains(value, doc, node);
154+}
155+
156 gchar const *Parameter::get_optiongroup(SPDocument const *doc, Inkscape::XML::Node const * node) const
157 {
158 ParamRadioButton const *param = dynamic_cast<ParamRadioButton const *>(this);
159@@ -247,6 +256,15 @@
160 return param->set(in, doc, node);
161 }
162
163+gchar const *Parameter::set_enum( gchar const * in, SPDocument * doc, Inkscape::XML::Node * node )
164+{
165+ ParamComboBox *param = dynamic_cast<ParamComboBox *>(this);
166+ if (!param) {
167+ throw Extension::param_not_enum_param();
168+ }
169+ return param->set(in, doc, node);
170+}
171+
172
173 /** Wrapper to cast to the object and use it's function. */
174 guint32
175
176=== modified file 'src/extension/param/parameter.h'
177--- src/extension/param/parameter.h 2012-02-29 01:16:51 +0000
178+++ src/extension/param/parameter.h 2013-04-30 18:52:28 +0000
179@@ -88,9 +88,11 @@
180 gchar const *get_enum(SPDocument const *doc, Inkscape::XML::Node const *node) const;
181
182 /** Wrapper to cast to the object and use it's function. */
183+ bool get_enum_contains(gchar const * value, SPDocument const *doc, Inkscape::XML::Node const *node) const;
184+
185+ /** Wrapper to cast to the object and use it's function. */
186 gchar const *get_optiongroup(SPDocument const * doc, Inkscape::XML::Node const *node) const;
187
188-
189 /** Wrapper to cast to the object and use it's function. */
190 bool set_bool(bool in, SPDocument * doc, Inkscape::XML::Node * node);
191
192@@ -101,6 +103,8 @@
193
194 gchar const *set_optiongroup(gchar const *in, SPDocument * doc, Inkscape::XML::Node *node);
195
196+ gchar const *set_enum(gchar const * in, SPDocument * doc, Inkscape::XML::Node *node);
197+
198 gchar const *set_string(gchar const * in, SPDocument * doc, Inkscape::XML::Node * node);
199
200 guint32 set_color(guint32 in, SPDocument * doc, Inkscape::XML::Node * node);
201
202=== modified file 'src/main.cpp'
203--- src/main.cpp 2013-04-06 14:17:29 +0000
204+++ src/main.cpp 2013-04-30 18:52:28 +0000
205@@ -148,6 +148,7 @@
206 SP_ARG_EXPORT_PS,
207 SP_ARG_EXPORT_EPS,
208 SP_ARG_EXPORT_PDF,
209+ SP_ARG_EXPORT_PDF_VERSION,
210 SP_ARG_EXPORT_LATEX,
211 #ifdef WIN32
212 SP_ARG_EXPORT_EMF,
213@@ -201,6 +202,7 @@
214 static gchar *sp_export_ps = NULL;
215 static gchar *sp_export_eps = NULL;
216 static gchar *sp_export_pdf = NULL;
217+static gchar *sp_export_pdf_version = NULL;
218 #ifdef WIN32
219 static gchar *sp_export_emf = NULL;
220 #endif //WIN32
221@@ -245,6 +247,7 @@
222 sp_export_ps = NULL;
223 sp_export_eps = NULL;
224 sp_export_pdf = NULL;
225+ sp_export_pdf_version = NULL;
226 #ifdef WIN32
227 sp_export_emf = NULL;
228 #endif //WIN32
229@@ -386,6 +389,12 @@
230 N_("Export document to a PDF file"),
231 N_("FILENAME")},
232
233+ {"export-pdf-version", 0,
234+ POPT_ARG_STRING, &sp_export_pdf_version, SP_ARG_EXPORT_PDF_VERSION,
235+ // TRANSLATORS: "--export-pdf-version" is an Inkscape command line option; see "inkscape --help"
236+ 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)"),
237+ N_("PDF_VERSION")},
238+
239 {"export-latex", 0,
240 POPT_ARG_NONE, &sp_export_latex, SP_ARG_EXPORT_LATEX,
241 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}"),
242@@ -1638,6 +1647,33 @@
243 }
244 (*i)->set_param_float("bleed", margin);
245
246+ // handle --export-pdf-version
247+ bool set_export_pdf_version_fail=true;
248+ const gchar *pdfver_param_name="PDFversion";
249+ if(sp_export_pdf_version) {
250+ // combine "PDF " and the given command line
251+ std::string version_gui_string=std::string("PDF ")+sp_export_pdf_version;
252+ try{
253+ // first, check if the given pdf version is selectable in the ComboBox
254+ if((*i)->get_param_enum_contains("PDFversion", version_gui_string.c_str())) {
255+ (*i)->set_param_enum(pdfver_param_name, version_gui_string.c_str());
256+ set_export_pdf_version_fail=false;
257+ } else {
258+ 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\".",
259+ sp_export_pdf_version);
260+ }
261+ } catch (...) {
262+ // can be thrown along the way:
263+ // throw Extension::param_not_exist();
264+ // throw Extension::param_not_enum_param();
265+ g_warning("Parameter or Enum \"%s\" might not exist",pdfver_param_name);
266+ }
267+ }
268+ // set default pdf export version to 1.4, also if something went wrong
269+ if(set_export_pdf_version_fail) {
270+ (*i)->set_param_enum(pdfver_param_name, "PDF 1.4");
271+ }
272+
273 //check if specified directory exists
274 if (!Inkscape::IO::file_directory_exists(uri)) {
275 g_warning("File path \"%s\" includes directory that doesn't exist.\n", uri);