Closed Bug 803831 Opened 7 years ago Closed 6 years ago

GCLI Command to open Profile Directory

Categories

(DevTools Graveyard :: Graphic Commandline and Toolbar, defect)

x86_64
Windows 7
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 33

People

(Reporter: rcampbell, Assigned: gl)

References

Details

Attachments

(1 file, 3 obsolete files)

It would be nice to have a command to open the user's Profile Directory in the file explorer of the user's operating system. Often useful when working on addons or modifying userChrome.css.

Could use "folder" for the parent command, "open" for a subcommand to open generic folders and openProfile to open the user's profile dir.

Components.utils.import("resource://gre/modules/osfile.jsm");
OS.Constants.Path.profileDir;

will get the user's profile directory from OS.File.

See https://gist.github.com/3923277 for an example of how to open a Windows Explorer on profile directory.
I like this idea ... we could use the same code as the Open Folder button in about:support.
New component triage. Filter on "Lobster Thermidor aux crevettes with a Mornay sauce"
Component: Developer Tools → Developer Tools: Graphic Commandline and Toolbar
Assignee: nobody → gabriel.luong
Attached patch 803831.patch (obsolete) — Splinter Review
Looking for some early feedback on the folder GCLI implementation. I am curious how "folder open <dir>" should work whether we require the user to provide an absolute path or a path from their home directory. 

Current implementation of "folder openProfile" reuses the code from openProfileDirectory() in https://mxr.mozilla.org/mozilla-central/source/toolkit/content/aboutSupport.js#571
Attachment #8382775 - Flags: feedback?(jwalker)
Comment on attachment 8382775 [details] [diff] [review]
803831.patch

Review of attachment 8382775 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good so far, thanks.

We'll need tests before we can commit this. browser_cmd_cookie.js [1] is a good example of some tests, and there are docs on helpers.audit in the source [2].

[1]: https://mxr.mozilla.org/mozilla-central/source/browser/devtools/commandline/test/browser_cmd_cookie.js
[2]: https://mxr.mozilla.org/mozilla-central/source/browser/devtools/commandline/test/helpers.js#982

::: browser/devtools/commandline/BuiltinCommands.jsm
@@ +2350,5 @@
> +    description: gcli.lookup("folderDesc")
> +  });
> +
> +  gcli.addCommand({
> +    name: 'folder open',

Are you missing an exec from this command?

@@ +2362,5 @@
> +    ]
> +  });
> +
> +  gcli.addCommand({
> +    name: 'folder openProfile',

Normally GCLI commands are lower case only.
Attachment #8382775 - Flags: feedback?(jwalker) → feedback+
Attached patch 803831.patch [WIP] (obsolete) — Splinter Review
Sorry, this has taken some time. I have been busy with school work, but here's the current WIP patch.

Implemented 'folder open' command. I made the home directory a default value for 'folder open'. 

Renamed 'folder openProfile' to 'folder openprofile'.

In showFolder(), added error checking when constructing the nsLocalFile object and checked that the provided folder exists.

Working on unit tests next.
Attached patch folder.patchSplinter Review
- 'folder open' opens a specified path, and opens the home directory if the path is left blank
- 'folder openprofile' opens the profile directory
- Added unit tests, which mainly checks if the command inputted is valid. I assumed there was no good way to test if a directory folder was actually opened on the build system.

try: https://tbpl.mozilla.org/?tree=Try&rev=306d24ed7bf4
Attachment #8382775 - Attachment is obsolete: true
Attachment #8390767 - Attachment is obsolete: true
Attachment #8440524 - Flags: review?(jwalker)
Comment on attachment 8440524 [details] [diff] [review]
folder.patch

Review of attachment 8440524 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/devtools/gcli/commands/folder.js
@@ +11,5 @@
> +                      .getService(Ci.nsIProperties);
> +
> +function showFolder(aPath) {
> +  let nsLocalFile = CC("@mozilla.org/file/local;1", "nsILocalFile",
> +                        "initWithPath");

Did you have a look at OS.File? I'm not sure if there is file.exists() with OS.File, but if there is then we should be using that in preference to nsLocalFile.

@@ +38,5 @@
> +    description: gcli.lookup("folderOpenDesc"),
> +    params: [
> +      {
> +        name: "path",
> +        type: { name: "string", allowBlank: true },

There is a file type. I hacked it together for a demo week, but I'm not sure if it's working well enough. IIRC we're not using it anywhere else. You should be able to do:

type: {
  name: 'file',
  filetype: 'directory',
  existing: 'yes'
}
Attachment #8440524 - Flags: review?(jwalker) → review+
Attached patch folder-file.patch (obsolete) — Splinter Review
I looked into OS.File, but it doesn't allow you to reveal the folder. I am not quite sure if you wanted to do OS.File.exist and then nsLocalFile.reveal().

In addition, I tried out the file type and have attached the patch that should be applied on top of the folder patch. The performance of the file parser isn't as great as the selection lookup. That said it would be useful to people learning the tool since it does provide predictions. The predictions doesn't work all the time unfortunately. If I was trying to open '/Users' and typed in 'Users', the helper will help me correct the path. However, it would not be able to find the '/Users' directory immediately until I completely typed 'Users'.

Give it a try and let me know what you think. I would ultimately like to incorporate the file type. I will look into to how we can improve the lookup when I get a chance.
Attachment #8441053 - Flags: feedback?(jwalker)
Comment on attachment 8441053 [details] [diff] [review]
folder-file.patch

Review of attachment 8441053 [details] [diff] [review]:
-----------------------------------------------------------------

The command line prompts are off. Not your fault, and given the rat-hole that getSpec turned into, I think we should go back to using 'string'. What do you think?
Attachment #8441053 - Flags: feedback?(jwalker)
Landing folder.patch for now. We will revisit getting the file type in a new bug.
Keywords: checkin-needed
Attachment #8441053 - Attachment is obsolete: true
https://hg.mozilla.org/integration/fx-team/rev/dc1141395851
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/dc1141395851
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 33
Product: Firefox → DevTools
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.