Re: [Openerp-community] [Merge] lp:~openerp-community/openobject-client/zehk_use-of-xdg-open into lp:openobject-client

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] [Merge] lp:~openerp-community/openobject-client/zehk_use-of-xdg-open into lp:openobject-client

Thierry Treyer
Hello everyone,

I didn't receive any comment since a while, so I come back.
I took into account your remarks and modify the code acordingly.

I would like to have some feedbacks.

Thanks
--
https://code.launchpad.net/~openerp-community/openobject-client/zehk_use-of-xdg-open/+merge/77333
Your team OpenERP Community is subscribed to branch lp:~openerp-community/openobject-client/zehk_use-of-xdg-open.

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] [Merge] lp:~openerp-community/openobject-client/zehk_use-of-xdg-open into lp:openobject-client

Naresh(OpenERP)
Review: Needs Fixing

Hello Thierry,

Thanks for your valuable contribution ! Just a few comments ! There's always a big risk associated when you make a big change, risk like loosing essential features, regressions etc. Its should be tested and well executed keeping in mind the previous functionalities offered. By loosing essential features I meant that Currently OpenERP allows you to define your own extensions from the extension manager options from the toolbar(GTK client) options/extension manager for eg: I would like to open/print *XYZ* extension files with firefox. Then OpenERP gives higher priority to the extensions defined by the user if found use them else go for the other openers according to extension type. So with your merge proposal this features gets lost.
secondly: OpenERP allows users to define their softpath(usually for pdf) and softpath_html(usuallyfor html types) i.e
If you want to use another PDF viewer or if you do not want to use the first one the OpenERP will find for you, you have to edit the OpenERP configuration file, normally located in ~/.openerprc and in the [printer] section edit the softpath and softpath_html parameter. This feature is also lost with your merge proposal.
Thirdly: can you please refactor your code please like remove the unused imports, unnecessary dict(self.openers) can be just a list as they all gives the same self._findOpener function.

please before making such changes please have a deep look at the existing system working and the features it provides. For eg: if you remove a line then first just think why was the line introduced. do I cover the intension in my new proposal etc.

Once again thanks for your work and patience ! Hope to see the improved merge soon.

Regards,
--
https://code.launchpad.net/~openerp-community/openobject-client/zehk_use-of-xdg-open/+merge/77333
Your team OpenERP Community is subscribed to branch lp:~openerp-community/openobject-client/zehk_use-of-xdg-open.

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp
Reply | Threaded
Open this post in threaded view
|

Re: [Openerp-community] [Merge] lp:~openerp-community/openobject-client/zehk_use-of-xdg-open into lp:openobject-client

Etienne Bagnoud
In reply to this post by Thierry Treyer
Hi,

I'm interested in sorting out this bug and have followed this thread. I think the rewrite of the module might be too much, but the original proposition was pretty good as it didn't introduce any regression (not modifying too much code) and worked.
xdg-open is part of XdgUtils which make it simpler for an application to use the correct program to open any given file or URL (http://portland.freedesktop.org/wiki/). OpenERP GTK Client uses xdg-open only for pdf file and an intern list for others application. When it doesn't find the right application it just crash. The first proposition keeps the original solution while adding the correct way of opening a file under operating systems that respect freedesktop and user's preferences.

Of course a rewrite is needed for this module, using a generic file opener (xdg-open) and not for others files shows a lack of understanding of what does xdg-open.

Maybe the first proposition should be merged in stable branch as first easy step : it correct a bug under Debian (and maybe others distributions) and does not introduce any regression and the last proposition should be made a basis for a new correct code for opening a file.

My 2 cents.
Etienne.
--
https://code.launchpad.net/~openerp-community/openobject-client/zehk_use-of-xdg-open/+merge/77333
Your team OpenERP Community is subscribed to branch lp:~openerp-community/openobject-client/zehk_use-of-xdg-open.

_______________________________________________
Mailing list: https://launchpad.net/~openerp-community
Post to     : [hidden email]
Unsubscribe : https://launchpad.net/~openerp-community
More help   : https://help.launchpad.net/ListHelp