[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