[referencer] Preliminary medline support (first python plugin)
jcspray at icculus.org
jcspray at icculus.org
Fri Jan 4 12:21:34 EST 2008
Quoting Aurélien Naldi <aurelien.naldi at gmail.com>:
> I renamed it (See the new version of the patch attached). I don't see a
> better way for the multiple document stuff right now, so I'm just
> letting it on the side. I also reworked the loading of python plugin to
> avoid having mandatory stubs of the functions the plugin doesn't
> implement (which allows to let the pubmed plugin unchanged and to skip
> the "resolve" function in the lyx plugin.
Looks fine. The explicit if() distinction between DOI|ARXIV|PUBMED
and DOCUMENT_ACTION for which functions to load isn't desirable long
term, but we can deal with that long term.
> I used shortName as identifier for the action. I have now added fields
> for the text and tooltip of the button. If the plugin doesn't have the
> DOCUMENT_ACTION capability, these fields are not used (and can thus be
> left undefined)
Fine. I notice in your following version of lyx.py you fixed the
capitalisation of the strings, very good.
>> * There need to be hooks for updating sensitivity of the action,
>> not just on whether any documents selected, but also on (for
>> example) whether lyx is running. It will be interesting to see
>> what performance consequences there are associated with jumping
>> into a python plugin everytime updating status UI.
>
> The other bib manager I used don't test if lyx is running. I guess that
> detecting it when actually trying to use it and putting a warning is
> enough for now. As I understand it the python interpreter is always
> running and you have pointers to python functions preloaded, so the
> performance cost should not be that bad if someone want to add this
> later.
I would like the sensitivity mechanism in before this is releasable,
since it's something in the Python/referencer API that we need to
define rather than an implementation detail.
I think it's true that the performance cost will be acceptable.
> I agree, defining a document as a python map seems enough for now, even
> if a real python object would be much nicer to have. It looks like the
> boost C++ libs have some tools to do this kind of stuff, but I'm too
> unexperienced with both python and C++ to have a real look at it now.
> I am a bit affraid that by doing so we may duplicate everything in
> memory, which can be annoying if many documents are selected.
Barring unforseen circumstances, I am going to implement true Python
Document class this weekend, which will be useful for this. There's a
nice example in Python in a Nutshell on how to do this :-)
> I can add another hook to return a list of the keys of selected
> documents, which is all what I need for the lyx plugin.
Don't do that, the API for a DOCUMENT_ACTION should simply return
documents, and leave it to the plugin to decide what information it
needs. A separate API for just the keys would be unnecessary
special-casing.
> I also think
> that the metadata resolvers could use a real acces to the document.
I agree.
>
> I may not have time to finish it before the end of the week, so here is
> my current patch, feel free to improve it in the meantime.
I may commit it to svn once I've done the python Document interface;
and adapt it to use that. I'll send you a note if I do.
> Comments ?
Style:
- Indentation: tabs only please. Seems like in this patch there is
some kind of mixture of tabs and spaces. It's no fun when these
things get inconsistent.
- Variable naming: abbreviate only when necessary. For instance,
there is no need to call a plugin pointer "pl" in
RefWindow::onPluginRun. Calling it plugin is harmless and reads more
easily.
The actionText_ and actionTooltip_ members of Plugin are unnecessary,
these values should be returned using getPluginInfoField. Therefore
they should be defined as {} in Plugin, with the real implementations
in PythonPlugin.
Constructing the plugin UI statically works for testing, but really
that needs to be dynamic. Disabling a plugin should remove its UI
elements. This should be accomplished by using Gtk::UIManager::add_ui
to merge in a plugin's UI, and storing the return value to use with
Gtk::UIManager::remove_ui when the plugin is disabled.
When constructing the GtkAction, prepend "_plugin_" to the shortname,
to avoid possible name conflicts.
This is encouraging, we're not too far from having this in a decent
state, and once the plugin API is well defined people hopefully can
scratch their own itches rather than needing me to do it ;-)
John
More information about the referencer
mailing list