GNOME Bugzilla – Bug 746441
Add a button in the file browsing panel to open the current folder
Last modified: 2015-03-22 19:49:26 UTC
Created attachment 299797 [details] [review] A first patch I implemented a patch to add a button in the toolbar of the file browsing panel to open the current folder. This tiny feature is very useful for me. The single problem have have for the moment is that the icon of the added button (open-document) is quite different than the other ones of the toolbar. Arnaud
Good idea, since the integrated file browser doesn't have a lot of possible actions (delete, rename files, etc).
Review of attachment 299797 [details] [review]: ::: src/file_browser.vala @@ +208,3 @@ toolbar.insert (get_jump_button (), -1); toolbar.insert (get_properties_button (), -1); + toolbar.insert (get_open_folder_button (), -1); I think it's better to have the properties at the end, since it's a different kind of action. @@ +276,3 @@ } + private ToolButton get_open_folder_button () In file_browser.vala, the vocabulary used is "directory", not "folder". So for consistency it'd be better to use "directory". @@ +280,3 @@ + ToolButton open_folder_button = new ToolButton (null, null); + open_folder_button.icon_name = "document-open"; + open_folder_button.tooltip_text = _("Open the active document directory"); The integrated file browser is not always on the active document's directory. So for the tooltip I'd write "Open the directory in a file manager". @@ +285,3 @@ + { + return_if_fail (_current_directory != null); + try Please add a blank line after return_if_fail(), so the code is more spaced out. @@ +287,3 @@ + try + { + AppInfo.launch_default_for_uri (_current_directory.get_uri (), null); You can maybe use Latexila.utils_show_uri() instead (available in the liblatexila). The timestamp of the clicked event should also be passed, so that gnome-shell doesn't show a notification saying that "Files ... is ready", it should show Nautilus directly. @@ +291,3 @@ + catch (Error err) + { + error ("%s", err.message); This will abort the program. So a warning() is better, with a small text describing that the error occurred for the open folder button in the file browser.
(In reply to arno_b from comment #0) > The single problem have have for the moment is that the icon of the added > button (open-document) is quite different than the other ones of the toolbar. It's ok, it's the right icon for that action. Some gnome apps now have symbolic icons (in black and white), but I don't really like that.
Created attachment 300074 [details] [review] A second patch Thx for the feedback. Here is a new patch. By the way, I also jump a lot between latexila and a terminal (git, svn, etc.). Are you interested to add another button to open a terminal in the current directory? I found no information on how to do that using gtk.
Review of attachment 300074 [details] [review]: A few more comments. Opening the directory in the terminal would be nice too. Maybe with one icon for the two actions, with a menu to choose whether to open the directory in a file manager or a terminal. But anyway this can be done with a later commit. I don't know how to open the terminal, but I think it's already done in gedit somewhere, so we can look at the code there. ::: src/file_browser.vala @@ +288,3 @@ + try + { + Latexila.utils_show_uri(this.get_screen (), _current_directory.get_uri (), Gdk.CURRENT_TIME); For the timestamp, it's better to take the Gdk.EventButton's time, like it is done here: https://git.gnome.org/browse/latexila/tree/src/document_view.vala#n265 Since the GdkEvent is not always available, Gdk.CURRENT_TIME is sometimes used. @@ +292,3 @@ + catch (Error err) + { + warning ("%s", "Unable to open the directory in a file manager"); Oh, the FileBrowser has a handle_error() function, it's even better to use it, so a message dialog is displayed.
Mmm, forget what I said about the Gdk.EventButton's time, the parameter is not available in the GtkToolButton::clicked signal. So using Gdk.CURRENT_TIME is fine.
Created attachment 300087 [details] [review] A third patch Ok I will search in the Gedit code. Here is a new version of the patch.
Pushed to the wip/latexila-next branch: https://git.gnome.org/browse/latexila/log/?h=wip/latexila-next The master branch is currently in string and code freeze for the 3.16 release. When I'll create the gnome-3-16 branch I'll merge the latexila-next branch into master. Thanks!