GNOME Bugzilla – Bug 719584
external tools plugin script for send to fpaste.org
Last modified: 2019-03-23 20:35:23 UTC
http://fpaste.org/58010/ I am not going to include support for fpaste commandline utility for now because their api has changed and the commandline utility is obsolete. Another reason is that i'd like this bash script to stay system agnostic as far as possible. I will support wget and curl for now. wget and curl are found on most linux installations. I have used sed (snippet taken from the web: commandlinefu.com) to urlencode the data when using wget. Tested for both selected text and whole documents, works fine. Though behaviour different if "input" option is changed. If I get a go ahead for the current bash script, i will finish up support for curl following it, shouldn't take long since it has --data-encoded flag that does urlencoding automagically.
I am aware that I have to add the script to the gedit codebase, patch coming up tomorrow!
Created attachment 263195 [details] [review] Add external tools plugin script: Send to fpaste The patch works fine, I tested it by trying to paste the script itself through my script and then compared the script to the raw paste and saw that the urlencode was escaping its own code incorrectly. I dont know how to fix this as I am not an expert in sed. sindhu@arch ~/tmp % diff --suppress-common-lines -y fpaste-raw-send-to-fpaste.sh gedit-send-to-fpaste.sh urlencoded_seltext=`echo "$GEDIT_SELECTED_TEXT" | sed 's/ / / | urlencoded_seltext=`echo "$GEDIT_SELECTED_TEXT" | sed 's/ /%2 urlencoded_contents=`echo "$contents" | sed 's/ / /g;s/!/!/g; | urlencoded_contents=`echo "$contents" | sed 's/ /%20/g;s/!/%2 fi \ fi > sindhu@arch ~/tmp %
Wouldn't it be better to just write this external tool in python instead of bash? The urlencode using sed is extremely fragile. You can also use the simple http client in python stdlib instead of relying on wget or curl to be installed on the system. Just a thought.
1:54 PM <ingu> nacho: was wondering if it would be a good idea to write a plugin for gedit 1:55 PM <ingu> nacho: the idea is a simple plugin that will allow a user to send selected text in gedit to any pastebin type of service 1:55 PM <nacho> that's always a good idea ;) 1:55 PM <nacho> ingu, what about using an external tool? 1:56 PM <ingu> nacho: like? 1:56 PM <nacho> ingu, gedit provides a plugin called external tools 1:56 PM <ingu> nacho: oh right! me will check it out 1:56 PM <nacho> this allows you to write bash scripts that are executed by pressing i.e some key combination 1:57 PM <nacho> ingu, if you write some nice external tool for doing this I am more than ok to include it in gedit directly 1:57 PM <ingu> awesome! I will work on it :) shouldnt be that hard
External tools are not limited to bash. Basically, anything that is a valid bash script (interpreter) can be used. So if you just use: #!/bin/env python3 at the top of the script, you'll have a python external tool.
Created attachment 264993 [details] [review] Add external tools plugin script send to fpaste Please review, thanks.
Review of attachment 264993 [details] [review]: ::: plugins/externaltools/data/send-to-fpaste.tool.in @@ +6,3 @@ +# Name=Send to fpaste +# Shortcut=<Shift><Super>p +# Input=nothing It would be better to have this tool use the selection (or whole document if there is no selection) as input. You can then simply read the document contents from STDIN. Like that it will work with any document, even remote or unsaved documents. @@ +26,3 @@ +final_data = json.loads(openfpaste) + +print("http://fpaste.org/" + final_data['result']['id']) It would be interesting to automatically save the url on the clipboard. The easiest way is probably to use the Gdk/Gtk clipboard API.
Created attachment 265898 [details] [review] Add external tools plugin script Send to fpaste (In reply to comment #7) > ::: plugins/externaltools/data/send-to-fpaste.tool.in > @@ +6,3 @@ > +# Name=Send to fpaste > +# Shortcut=<Shift><Super>p > +# Input=nothing > > It would be better to have this tool use the selection (or whole document if > there is no selection) as input. DONE. > You can then simply read the document contents > from STDIN. Like that it will work with any document, even remote or unsaved > documents. DONE. > @@ +26,3 @@ > +final_data = json.loads(openfpaste) > + > +print("http://fpaste.org/" + final_data['result']['id']) > > It would be interesting to automatically save the url on the clipboard. The > easiest way is probably to use the Gdk/Gtk clipboard API. DONE.
Review of attachment 265898 [details] [review]: ::: plugins/externaltools/data/send-to-fpaste.tool.in @@ +8,3 @@ +# Input=selection-document +# Comment=Paste selected text or current document to fpaste +# Languages= Sorry for noticing before, but you should move all of the Gedit Tool stuff (these comments) into the .desktop.in file. The build will automatically merge the desktop file and the tool file together when you make. @@ +14,3 @@ +from gi.repository import Gtk, Gdk + +lang="text" Would be nice to support some more langs? @@ +20,3 @@ + +if selected_text == None: + text_to_paste = sys.stdin.read() Use 4 spaces, not a tab (PEP8) @@ +30,3 @@ +paste_url= "http://fpaste.org/" + final_data['result']['id'] + +print (paste_url) No space before (. Maybe we should also print something like: "(copied to clipboard)" on the next line so it's obvious that you can just paste it afterwards directly.
Created attachment 265951 [details] [review] Add external tools plugin script Send to fpaste (In reply to comment #9) > Review of attachment 265898 [details] [review]: > Sorry for noticing before, but you should move all of the Gedit Tool stuff > (these comments) into the .desktop.in file. The build will automatically merge > the desktop file and the tool file together when you make. Done. > @@ +14,3 @@ > +from gi.repository import Gtk, Gdk > + > +lang="text" > > Would be nice to support some more langs? Filed a patch here: https://bugzilla.gnome.org/show_bug.cgi?id=721946 But it's not working, I have no clue why. > Use 4 spaces, not a tab (PEP8) Done. > @@ +30,3 @@ > +paste_url= "http://fpaste.org/" + final_data['result']['id'] > + > +print (paste_url) > > No space before (. Maybe we should also print something like: "(copied to > clipboard)" on the next line so it's obvious that you can just paste it > afterwards directly. Done.
Review of attachment 265951 [details] [review]: ::: plugins/externaltools/data/Makefile.am @@ +4,3 @@ build.tool.in \ + remove-trailing-spaces.tool.in \ + send-to-fpaste.tool.in Could you please rebase against master, this part has changed slightly on master. ::: plugins/externaltools/data/send-to-fpaste.tool.in @@ +8,3 @@ +selected_text = os.getenv('GEDIT_SELECTED_TEXT') + +print(lang) Remove debug print @@ +10,3 @@ +print(lang) + +if selected_text == None: Use "selected_text is None" is more correct than == to compare to None @@ +13,3 @@ + text_to_paste = sys.stdin.read() + +text_to_paste = selected_text Please, please test this before submitting. This is obviously wrong and so won't work on a document where no text is selected @@ +21,3 @@ +paste_url = "http://fpaste.org/" + final_data['result']['id'] + +print(paste_url + " has been copied to clipboard.") Extra empty line after this. Also, this is out of scope maybe for your patch, but as a note, it would be interesting if this could be translated. It's probably not very difficult to support.
Created attachment 266505 [details] [review] Add external tools plugin script Send to fpaste (In reply to comment #11) > Review of attachment 265951 [details] [review]: > > ::: plugins/externaltools/data/Makefile.am > @@ +4,3 @@ > build.tool.in \ > + remove-trailing-spaces.tool.in \ > + send-to-fpaste.tool.in > > Could you please rebase against master, this part has changed slightly on > master. Done. > ::: plugins/externaltools/data/send-to-fpaste.tool.in > @@ +8,3 @@ > +selected_text = os.getenv('GEDIT_SELECTED_TEXT') > + > +print(lang) > > Remove debug print Done. > @@ +10,3 @@ > +print(lang) > + > +if selected_text == None: > > Use "selected_text is None" is more correct than == to compare to None Done. > @@ +13,3 @@ > + text_to_paste = sys.stdin.read() > + > +text_to_paste = selected_text > > Please, please test this before submitting. This is obviously wrong and so > won't work on a document where no text is selected Done. Tested the script in Gedit stable, works with selected text (selected text is pasted) and unselected (whole document is pasted). Unable to test the script in Gedit master, as I get this error: http://fpaste.org/69120/38990825/raw/ > @@ +21,3 @@ > +paste_url = "http://fpaste.org/" + final_data['result']['id'] > + > +print(paste_url + " has been copied to clipboard.") > > Extra empty line after this. Also, this is out of scope maybe for your patch, > but as a note, it would be interesting if this could be translated. It's > probably not very difficult to support. I do not know how to support translation, could you give me some general instructions and/or which code sample I should be looking at?
Review of attachment 266505 [details] [review]: This looks great!
Just one minor issue, for a second patch after this one is merged. I'd like the text to be able to be translated. For that probably you need to rename the file to .in.in and add the makefile rule.
(In reply to comment #12) > I do not know how to support translation, could you give me some general > instructions and/or which code sample I should be looking at? I'm not sure exactly either. It would involve something like: import gettext try: gettext.bindtextdomain("@GETTEXT_PACKAGE@", "@LOCALEDIR@") _ = lambda s: gettext.dgettext("@GETTEXT_PACKAGE@", s) except: _ = lambda s: s Then you'd need to process this file with configure so it substitutes @GETTEXT_PACKAGE@ and @LOCALEDIR@, and then you'd do: print(_('{0} has been copied to clipboard.').format(paste_url)) You'd also need to add the file to POTFILES.in I'd suggest to commit the tool and work on this later (if you want), or otherwise I can try to implement the translation.
(In reply to comment #13) > Review of attachment 266505 [details] [review]: > > This looks great! Pushed to master in 349201e. Available at: https://git.gnome.org/browse/gedit/commit/?id=349201e4bbb2d84934915cf293d6122894feb5d1
Created attachment 266702 [details] [review] For translation, rename send-to-fpaste.tool.in to *.in.in (In reply to comment #14) > Just one minor issue, for a second patch after this one is merged. I'd like the > text to be able to be translated. For that probably you need to rename the file > to .in.in and add the makefile rule. Done.
(In reply to comment #15) > I'd suggest to commit the tool and work on this later (if you want), or > otherwise I can try to implement the translation. I'd like to try, I will submit a patch soon.
Minor issue with if lang is None has been fixed in cd33434. Available at: https://git.gnome.org/browse/gedit/commit/?id=cd33434bf46e13cbe05976d40870a3019a00c29a
this was committed no? I am closing the bug, please reopen if I am missing something...