amites, Monday 07 May 2012 à 22:12
|
|
Subscription date : 04 April 2012
Messages : 11
|
Wrote a patch for the gmail applet to open the message being clicked on directly vs the gmail inbox. Curious where this is being tracked so I know where to submit a patch?
I posted this same question on the launchpad for cairo-dock-plugin-extras including sourcecode at
https://answers.launchpad.net/cairo-dock-plug-ins-extras/+question/196162 |
Subscription date : 05 August 2009
Messages : 285
|
First, create your Launchpad account. Then refer to this section on the Wiki, specifically the "About branches" part. Basically you have to branch the "Plugin Extras" repository. Then you make your changes on the code, commit, and push it. And then you ask for "Merge proposal" on the Launchpad website.
This is the general framework, if you need help on the specific parts, i.e., on the specific commands, or a better explanation, do not hesitate to ask again. |
amites, Monday 07 May 2012 à 23:34
|
|
Subscription date : 04 April 2012
Messages : 11
|
Thank you Eduardo -- might take a couple days to get to it as I'm unfamiliar with working with Bazaar -- same process as git though contributed to a number of OS projects using github etc...
already have a launchpad account so no biggy there |
Subscription date : 05 August 2009
Messages : 285
|
Anytime. Sure, take your time. Thanks for your contribution. |
Subscription date : 30 November 2007
Messages : 17118
|
yep, making your own branch is a convenient to share your modifications.
although for a patch, you can also post a diff file as you did, thanks a lot by the way
@Eduardo: as you are our Python expert, can you please review this patch ?
also since we already have the mail url, why can't we open it directly ? |
amites, Tuesday 08 May 2012 à 04:15
|
|
Subscription date : 04 April 2012
Messages : 11
|
The way it was setup the url has hardcoded to goto the inbox
the regex is there to open the url through the inbox vs the all mail display |
Subscription date : 05 August 2009
Messages : 285
|
@Fab, yes, the patch is cool, works, but. The answer for your question is, yes, it is possible to use the link to open the e-mail directly, but the point is. Once the applet uses os.popen('x-www-browser') to open links, it does not redirects from this link to the "right" one. Let me give an example:
Let's say the link to the new e-mail is: http://mail.google.com/mail?account_id=YOUR_EMAIL@gmail.com&message_id=EMAIL_ID&view=conv&extsrc=atom , this link, when opened on the browser should be redirected to a link formated as https://mail.google.com/mail/?fs=1&source=atom#inbox/EMAIL_ID , and this last link would take you directly to the e-mail, not to the inbox. But if you open the first link with os.popen('x-www-browser') it does not work as such, and you are redirected to the inbox, i.e., https://mail.google.com/mail/?account_id=YOUR_EMAIL@gmail.com#inbox
So, the problem in reality is that, instead of using the Standard Lib Webbrowser, it was coded with os.popen('x-www-browser') . So, on the patch, Amits rewrote the URL direct on the format https://mail.google.com/mail/?fs=1&source=atom#inbox/EMAIL_ID by extracting the EMAIL_ID and puting it back on the final URL.
If, instead, we use it works like a charm. The trade-off here is, we do not import re, and import webbrowser. So, from the patch proposed by amites, I'm going to simplify the open_mail method to something like this:
def open_mail(self, menu, mail=None):
""" Opens the mail URL """
try:
link = mail['link']
webbrowser.open(link)
except webbrowser.Error:
os.popen('x-www-browser https://mail.google.com/mail')
I do not use to catch this error , I use webbrowser on other applets, and no problems so far, anyway ...
I am going to commit, and propose the merge. I am going to add a note with credits to Amits too. Any suggestion, let me know. |
matttbe, Tuesday 08 May 2012 à 14:29
|
|
Subscription date : 24 January 2009
Messages : 12573
|
@Eduardo & amites: thank you for your help
I agree with Eduardo. When the patch is ready, I merge your branch with the trunk  |
amites, Tuesday 08 May 2012 à 18:25
|
|
Subscription date : 04 April 2012
Messages : 11
|
The reason I rewrote the URL is that it opens the message through the All Mail view. The effect for the user is that when you archive the message it doesn't take you to your inbox but leaves the message on screen and says that the message has been archived.
I understand not wanting to import additional packages so I pulled the branch fresh from launchpad and rewrote my approach to elimiante the extra dependency. Diff below.
$ bzr diff
=== modified file 'Gmail/Gmail.py'
--- Gmail/Gmail.py 2012-03-12 10:22:04 +0000
+++ Gmail/Gmail.py 2012-05-08 16:22:55 +0000
@@ -54,7 +54,7 @@
menu_item.set_image(gtk.image_new_from_file('./img/menu-gmail.png'))
menu_item.get_children()[0].set_markup(string)
menu_item.url = mail['link']
- menu_item.connect('activate', self.open_mail)
+ menu_item.connect('activate', self.open_mail, mail)
self.append(menu_item)
menu_item.show()
# add a separator if mail is not last in list
@@ -72,7 +72,11 @@
""" Opens the mail URL """
- os.popen('x-www-browser https://mail.google.com/mail')
+ try:
+ link = mail['link'].replace('&view=conv', '').replace('&extsrc=atom', '').replace('message_id=', 'fs=1&source=atom#inbox/')
+ os.popen('x-www-browser %s' % link.replace('&', '\&'))
+ except KeyError:
+ os.popen('x-www-browser https://mail.google.com/mail')
The advantage to this approach is that it remains fairly dynamic removing extra variables piecemeal so if they appear in a different order it will not break, and avoids the dependency on webbrowser. I understand if you want to keep it in place, the approach will work either way. |
Subscription date : 05 August 2009
Messages : 285
|
@amites, actually, it is not an issue to import more modules, I pointed out "importing webbrowser instead re" just as a note, I do not even know how they are comparable in terms of size, etc, it is not a problem. My main point was to simplify the code, since we can rely on a std lib, and we already have the link directly to the e-mail, why tweak on this URL to make it fits with an os.popen hack?
It is a detail? Yes, it is. Both ways are going to work and we are going to be happy? Yes . On a situation in which two solutions are comparable (in terms of time, resources, etc), which one is simpler to understand when look at the code? Personally I think like that. Well, the main contribution here is that the users now can go directly to the e-mail, instead of to the inbox. Thank you for that.
We can even merge both ways, first using webbrowser.open, and, as a fallback, using the approach with os.popen. "Defensive Programming".
Thanks for more this effort on improving your patch. How is it going with bzr? Let me know if you get stuck there. Im not skilled on bzr, so probably Im going to redirect your question to Matttbe, or Fab |
amites, Tuesday 08 May 2012 à 20:45
|
|
Subscription date : 04 April 2012
Messages : 11
|
@Eduardo understood, it's personal preference for going through the inbox interface as opposed to all mail -- I can always make a new branch to keep my own version of the gmail applet
getting a bit more used to bzr -- still wrapping my mind around their version of branches with git a branch is stored in the same directory whereas bzr seems to make branches separate
I'll likely keep with posting diff outputs for the near future -- have my eye set on the note taking app -- want to show folders as the sub-menu with notes as a 2nd sub-menu vs the display of all notes now -- just looks cluttered, though I'm not in a hurry to do so
one thing I like about working with open source projects is the chance to integrate opposing view points and ideas -- great chance to gain skill as a programmer
Thank you for your ongoing contributions to Glx-Dock -- has made my Gnome 3 desktop much more friendly and effecient |
Subscription date : 30 November 2007
Messages : 17118
|
@Eduardo: thanks for the details, I understand
indeed less code implies less bugs, easier maintenance.
so if both methods are equivalent (it seems to me they are), let's just take the easy way, we just want it to work.  |
|