After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 611209 - ability to open a folder path from the Alt+F2 launcher and the activities window
ability to open a folder path from the Alt+F2 launcher and the activities window
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
2.29.x
Other Linux
: Normal enhancement
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2010-02-26 18:21 UTC by Jean-François Fortin Tam
Modified: 2010-03-08 23:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[runDialog] Support opening folders (4.08 KB, patch)
2010-02-26 19:23 UTC, Florian Müllner
none Details | Review
add ability to open file with default app in runDialog (1.48 KB, patch)
2010-02-26 19:33 UTC, Maxim Ermilov
reviewed Details | Review
add ability to open file/directory with default app in runDialog (1.49 KB, patch)
2010-02-26 21:39 UTC, Maxim Ermilov
rejected Details | Review
[runDialog] Support opening files and folders (4.21 KB, patch)
2010-02-26 21:46 UTC, Florian Müllner
committed Details | Review

Description Jean-François Fortin Tam 2010-02-26 18:21:38 UTC
If I want to acces /tmp/foo or /var/log/, with metacity's Alt+F2 dialog, I can do that. I'd like being able to do the same thing in gnome-shell; typing paths instead of having to launch nautilus to do so.
Comment 1 Florian Müllner 2010-02-26 19:23:50 UTC
Created attachment 154785 [details] [review]
[runDialog] Support opening folders

Support completion on folders in the run dialog and launch the file
manager when activated. Assume a folder relative to the home directory
if execution as command fails and the input doesn't start with a slash.
Comment 2 Maxim Ermilov 2010-02-26 19:33:47 UTC
Created attachment 154787 [details] [review]
add ability to open file with default app in runDialog

Bit more general patch
Comment 3 Colin Walters 2010-02-26 21:12:35 UTC
Review of attachment 154787 [details] [review]:

::: js/ui/runDialog.js
@@ +313,3 @@
+                if (str.charAt(0) == '~') {
+                let str = command;
+            if (command.indexOf(' ') == -1 && !inTerminal) {

I don't understand this; won't it fail if the shell isn't being run from your home directory?  I think we need to call g_get_home_dir().

@@ +317,3 @@
+                if (str.charAt(0) == '~') {
+                let str = command;
+            if (command.indexOf(' ') == -1 && !inTerminal) {

If we're opening a regular file, why are we checking for CAN_EXECUTE?
Comment 4 Colin Walters 2010-02-26 21:12:35 UTC
Review of attachment 154787 [details] [review]:

::: js/ui/runDialog.js
@@ +313,3 @@
+                if (str.charAt(0) == '~') {
+                let str = command;
+            if (command.indexOf(' ') == -1 && !inTerminal) {

I don't understand this; won't it fail if the shell isn't being run from your home directory?  I think we need to call g_get_home_dir().

@@ +317,3 @@
+                if (str.charAt(0) == '~') {
+                let str = command;
+            if (command.indexOf(' ') == -1 && !inTerminal) {

If we're opening a regular file, why are we checking for CAN_EXECUTE?
Comment 5 Florian Müllner 2010-02-26 21:25:19 UTC
Mmmh, tough:

I like allowing '~' for $HOME, and launching arbitrary files makes sense as well.

But:
- I don't like that ctrl-enter skips the whole files/folder stuff (it's nonsense
  to "run" files in a terminal, but we should not punish users who hit ctrl-enter
  out of a habit - just ignore the modifier if it doesn't make sense)

- I very much prefer Gio over xdg-open (in fact, removing the GLib.FileTest.IS_DIR
  from my patch "magically" makes it work for all files)

- I prefer my approach of trying to execute the command and falling back to
  interpreting the input as files better than using file attributes to decide
  whether it's a command or a regular file - there are programs which set the
  execution bit on e.g. images/video; it's stupid and wrong, but it's something
  we should deal with. "Why can I launch some images with Alt-F2 but not others?"
  is not a question users should have to ask themselves if we can avoid it

- we should enable completion in $HOME (note that the completion in my patch only
  works within one "level" (I mean: 'Des<tab>' completes to $HOME/Desktop, but
  'Desktop/som<tab>' does _not_ complete $HOME/Desktop/someFile. That should be
  fixed obviously)

So I'd tend to update my patch (allow launching any file and some cleanup - prepending "file://" to the path is pretty ugly and should probably done using the GFile api ... of course, I'm not exactly impartial on this matter ;)
Comment 6 Florian Müllner 2010-02-26 21:27:31 UTC
(In reply to comment #4)
> If we're opening a regular file, why are we checking for CAN_EXECUTE?

It's used to determine whether it's a regular file or a command.
Comment 7 Maxim Ermilov 2010-02-26 21:39:54 UTC
Created attachment 154802 [details] [review]
add ability to open file/directory with default app in runDialog

> If we're opening a regular file, why are we checking for CAN_EXECUTE?

In case It is a path to executable file. (for example /usr/bin/gnome-calculator)
Comment 8 Florian Müllner 2010-02-26 21:46:42 UTC
Created attachment 154803 [details] [review]
[runDialog] Support opening files and folders

Updated patch to work with any files. '~' at the start of the input is replaced with $HOME.
Comment 9 Colin Walters 2010-03-05 18:18:07 UTC
Review of attachment 154803 [details] [review]:

::: js/ui/runDialog.js
@@ +329,3 @@
+                if (input.charAt(0) == '/') {
+                let path = null;
+                // Mmmh, that failed - see if @input matches an existing file

This synchronous filesystem access only happens when the user presses Return, correct?  In that case it's probably OK, but we have to be really careful about doing this kind of thing in completions.

@@ +340,3 @@
+                    // We are only interested in the actual error, so parse
+                    //that out.
+                    let m = /.+\((.+)\)/.exec(e);

Hmm, this is a little ugly depending on the exact format of the error message; I don't think we can rely on translations using parenthesis.

@@ +346,3 @@
+                    this._errorBox.show();
+                    // preferred_size change. Without this, message will show with delay
+                    this._errorBox.get_parent().queue_relayout();

Why?  I deleted this line and things seem to work fine here.
Comment 10 Dan Winship 2010-03-06 14:22:32 UTC
(In reply to comment #9)
> +                    // We are only interested in the actual error, so parse
> +                    //that out.
> +                    let m = /.+\((.+)\)/.exec(e);
> 
> Hmm, this is a little ugly depending on the exact format of the error message;
> I don't think we can rely on translations using parenthesis.

Note that the code already does that, he just moved it somewhere else. Once bug 602516 and bug 591480 are done we can make pretty error messages ourselves based on the GError domain/code.
Comment 11 Florian Müllner 2010-03-07 23:44:42 UTC
(In reply to comment #9)
> This synchronous filesystem access only happens when the user presses Return,
> correct?

Yes. The only part of the patch which relates to completion is the addition of HOME to the directory array in _init(); I'm tempted to remove that part from the patch as it does not work for subfolders.


> @@ +346,3 @@
> +                    this._errorBox.show();
> +                    // preferred_size change. Without this, message will show
> with delay
> +                    this._errorBox.get_parent().queue_relayout();
> 
> Why?  I deleted this line and things seem to work fine here.

No idea. The only change here is the code's indentation level ...
Comment 12 Colin Walters 2010-03-08 19:06:45 UTC
(In reply to comment #11)
> 
> > @@ +346,3 @@
> > +                    this._errorBox.show();
> > +                    // preferred_size change. Without this, message will show
> > with delay
> > +                    this._errorBox.get_parent().queue_relayout();
> > 
> > Why?  I deleted this line and things seem to work fine here.
> 
> No idea. The only change here is the code's indentation level ...

Ok when you commit can you remove it and add a note "Removed workaround for relayout" or something?  That line originated from zaspire's original CSS port but I just don't see why it'd be necessary.
Comment 13 Florian Müllner 2010-03-08 23:07:49 UTC
Attachment 154803 [details] pushed as 62ca7fb - [runDialog] Support opening files and folders