GNOME Bugzilla – Bug 170767
Make ssconvert accept options for text import/export details
Last modified: 2007-11-02 04:09:26 UTC
Version details: debian 1.4.3-1 Distribution/Version: Debian Sid As gnumeric doesn't remember settings used for "text export" (for instance, the encoding defaults to $LANG, separator to space, etc, so we need to specify export settings each time). the CLI utility ssconvert could be used but it doesn't know how to handle options for a given exporter (say Gnumeric_stf:stf), so it just sticks to defaults. Please make ssconvert to accept options for(ex|im)porters or gnumeric to remember settings at least for a session. Thanks for the good work !
Created attachment 97928 [details] [review] Add support to parse command line options to file savers. This patch is for goffice. A patch for gnumeric/ssconvert using this will follow.
Created attachment 97930 [details] [review] Implement option parsing for Text File (stf_assistant) exporter. This patch should be combined with the previous patch for goffice. ps. This is my first submitted patch ever. Please tell me if things can/should be improved.
Created attachment 97932 [details] [review] Implement option parsing for Text File (stf_assistant) exporter. Oeps, svn diff / me did not include the new files in the previous patch. Sorry, this patch does.
This looks very promising. It might even get in before 1.8.0 There are a few things that worries me: 1. Moving gnm_init early. That means that nothing in the init can depend on options. 2. IS_WBC_GTK (context->impl). I need to think about that one. 3. Should we name options some way consistent way like "stf-foo"? Minor issues: 1. Global static variables like eol_type. It should not be too hard to toss those into a structure and dynamically allocate it. 2. Property specs should include GSF_PARAM_STATIC. 3. Name space -- all this looks like it goes into libspreadsheet and thus should be put in gnm_ name space or something. We have aspirations of one day being a real library. 4. (GValue *) stf_file_saver_get_option_group (stf_file_saver)) That cast looks invalid. And unneeded.
Created attachment 97977 [details] [review] Add support to parse command line options to file savers. Patch for goffice, replacing previous patch to address issues raised by Morten
Created attachment 97978 [details] [review] Implement option parsing for Text File (stf_assistant) exporter. This patch for gnumeric (in combination with the new patch for goffice) adresses most worries and issues raised by Morten. Worries: 1. Moving gnm_init early. => I moved it back again to the orignal location. Now using g_option_context_parse twice to first parse application options, then gnm_init, and then parse again for exporter options. 2. IS_WBC_GTK (context->impl). => I did not fix this one. I copy/pasted this verbatim for the function it replaces (stf.c/stf_write_workbook). I agree however with Morten that it should be done differently. As commented in the code, this code should not be in a file_saver, but somewhere in gui-file.c 3. Consistant option naming. => Although GOption automatically prefixes group options in case of conflicting options, I agree it is better to prefix anyway. Now using stf-foo. Minor issues: 1. I put all static variables in one static struct. I'm not sure this is the best solution though. I could not find any examples of programs that use GOption parsing without static variables. Is it really worthwhile to avoid static variables (I'm no C-expert (yet?), so I really don't know)? 2. GSF_PARAM_STATIC: fixed. 3. Name space: I put a GNM/gnm/Gnm in front of almost everything. 4. GValue: fixed.
I like the approach in ssconvert, and the extension in the saver is in the right place (eventually both should generalize to the openers too). However, the direct use of GOptionGroup only solves part of the problem. The use of the static structure is a more general symptom. We've had requests to remember the settings used in stf between invocations. eg a) save to stf b) customize the settings c) re-save later, we forgot the settings. It seems like what we really want is a GObject based state blob that can load/store the parameters. With an extra flag on a param we could generate a GOptionGroup and point it at the state object itself (removing the global). Something like GOG_PARAM_PERSISTENT but for cmd line options. This would also cleanup the duplication of information we're seeing. Thoughts ?
Hib Eris: thanks for the patch. I think we'll take it from here (when we figure out the details).
I have this under control.
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report. I think that's it. I ended up with --export-options="eol=unix format=raw ...". We can use that same code for the pdf exporter for things like "paper=a4".