Hi Martin,
Am 16.09.2014 um 11:34 schrieb Martin Metzker:
Hello list!
I think I have a viable patch now, for command line provided python scripts
in dia. Going over the changes:
... moved the definition of struct _PluginInfo from lib/plug-ins.c to
lib/plug-ins.h, because I need this struct in app/app_procs.c.
not ok. To avoid this please use dia_plugin_get_symbol().
Using dia_plugin_get_symbol() and dia_plugin_get_name() now. No changes to
lib/plug.ins.{c,h} anymore.
ok.
... modified app/app_procs.c app_init() to include the -i option and stores
the provided filename in a local variable python_script.
ok, though I would prefer another short option, maybe -r and --run-script
instead of --python?
Done.
ok.
... added a block if(python_script) which checks if the python plug-in is
loaded, and does run_script_oneshot(python_script) when sensible.
Extended functionallity: run_script_oneshot() still exists as described and
is still being used during initialization. For running the script provided
via the command line, there is now run_script_w_context(), which makes the
variables files, export_file_name, export_file_format, input_directory and
output_directory accessible from within python.
This sounds complicated and I'm not sure I like the design to arbitrarily
push some variables into the python namespace.
To accomplish this, the
code block in app/app_progs.c creates a GHashTable with these variables
(files is collapsed to a ,-separated list), which is passed to
run_script_w_context(), which propagates the information to the python
interpreter before calling run_script_oneshot().
Here I'm very uncertain, starting with the extra function, the huge block
of code changes in app_init() and the ad hoc API via GHashTable between
Dia's core and the Python plug-in. Maybe there should be an official PyDia
API instead, to accomplish the same goal?
While I don's share your big vision of multiple Python threads in Dia, I
think these additions are moving us away from the goal to allow the
oneshot script to interact with the GUI.
IMO the patch is not too big to be discussed (and finally be applied) in
one piece.
I would be happy if that is still the case.
Sorry, not quite. The patch providing the initial functionality was closer
to inclusion, than this one is.
There are minor coding style glitches (tab size, placement of braces) but
I can fix these before commiting.
I modified all the code I touched. There seems to be a little inconsistency
with indentation.
There definitely is - between plug-ins and core, somethimes even with-in
single files, or even functions. But multiple line reformating should be
separated from critical content addtions, IMHO.
I kept to the dominant style of app/app_progs.c which is
3 spaces for function bodies and an additional 2 spaces for each {. My code
layout should be at least consitently broken.
For app/app_procs.c and most of the core I'd say it is indendation of 2;
braces at the same line as 'if' or 'else'. With plug-ins/python/python.c
it's indent 4 AFAICT.
In addition, at its very beginning, this patch includes
- ef-export_func(diagdata, ctx, outfname, infname, size);
+ ef-export_func(diagdata, ctx, outfname, infname, (void *) size);
which fixes a compile warning that kept annoying me.
This warning shows a design problem and should not be brushed under the
carpet with a patch unrelated to that issue. I also like code which
compiles cleanly, but this is one of at least two warnings I want to keep
until the underlying problem is solved.
If I would have to commit the patch I'd separte it into two parts:
1) Initial version of running scripts from the commandline
- Basically what was reviewed before, with the requested changes.
- Additionally a simple but somehow useful test script for the
new functionality
2) Extended parameter support for command line scripts
- As I said I'm uncertain at the moment how to do it. I would
probably experiment with differnt approaches, like using sys.argv;
connecting to DiaApplication signals or ...
- If it is to be done the way you propose it should be at least
factored into it's own function.
HTH,
Hans
Hans at Breuer dot Org ---
Tell me what you need, and I'll tell you how to
get along without it.-- Dilbert
___
dia-list mailing list
dia-list@gnome.org
https://mail.gnome.org/mailman/listinfo/dia-list
FAQ at http://live.gnome.org/Dia/Faq
Main page at http://live.gnome.org/Dia
