663075 - FileUtils.jsm should have an easy way to create an nsILocalFile with a path

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Comment 1

[Geoff Lankow (:darktrojan)]

Attachment: patch

Comment 2

[Philipp von Weitershausen [:philikon]]

Comment on attachment 538231 [details] [diff] [review]
patch

Drive-by review...

>+  getFileWithPath: function FileUtils_getFileWithPath(aPath) {
>+    let file = Cc ["@mozilla.org/file/local;1"].createInstance (Ci.nsILocalFile);

Nit: get rid of the spaces before [ and (

>+add_test(function test_getFileWithPath() {
>+  let testpath = FileUtils.getFile("ProfD", ["test"]).path;
>+  let file = FileUtils.getFileWithPath(testpath);
>+  do_check_true(file instanceof Components.interfaces.nsILocalFile);
>+  do_check_eq(file.path, testpath);

You could also do file.equals(otherfile) where otherfile is the one created via FileUtils.getFile().

Comment 3

[Shawn Wilsher :sdwilsh]

Comment on attachment 538231 [details] [diff] [review]
patch

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

::: toolkit/mozapps/shared/FileUtils.jsm
@@ +138,5 @@
>      }
>      stream.close();
> +  },
> +
> +  getFileWithPath: function FileUtils_getFileWithPath(aPath) {

I think I'd rather expose an object that does the magic here:
let file = new FileUtils.File("/path/to/file");
instead of:
let file = FileUtils.getFileWithPath("/path/to/file");

Feels more JavaScripty and less XPCOMy to me at least.  Thoughts?

Comment 4

[Geoff Lankow (:darktrojan)]

Suits me, I'll do that.

Comment 5

[Geoff Lankow (:darktrojan)]

Attachment: patch

Not sure why I didn't attach this earlier. Hmm.

Comment 6

[Shawn Wilsher :sdwilsh]

Comment on attachment 544944 [details] [diff] [review]
patch

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

Sorry this has taken so long!  Should be a snappy review with these comments addressed.  Then this will need sr from :rs

::: toolkit/mozapps/shared/FileUtils.jsm
@@ +159,5 @@
>      }
>      stream.close();
> +  },
> +
> +  File: function FileUtils_File(aPath) {

I think you actually just want to make `File` a `Components.Constructor` object: https://developer.mozilla.org/en/Components.constructor

::: toolkit/mozapps/shared/test/unit/test_FileUtils.js
@@ +189,5 @@
> +  let testpath = testfile.path;
> +  let file = new FileUtils.File(testpath);
> +  do_check_true(file instanceof Components.interfaces.nsILocalFile);
> +  do_check_true(file.equals(testfile));
> +  do_check_neq(file, testfile);

I don't think this check is useful.

Comment 7

[Geoff Lankow (:darktrojan)]

Attachment: patch v3

(In reply to Shawn Wilsher :sdwilsh from comment #6)
> I think you actually just want to make `File` a `Components.Constructor`
> object: https://developer.mozilla.org/en/Components.constructor

Ooh, I just learned a new trick!

> ::: toolkit/mozapps/shared/test/unit/test_FileUtils.js
> @@ +189,5 @@
> > +  let testpath = testfile.path;
> > +  let file = new FileUtils.File(testpath);
> > +  do_check_true(file instanceof Components.interfaces.nsILocalFile);
> > +  do_check_true(file.equals(testfile));
> > +  do_check_neq(file, testfile);
> 
> I don't think this check is useful.

I'm assuming you mean just the last line and splinter gave 4 lines of context just to be confusing, right?

Comment 8

[Geoff Lankow (:darktrojan)]

Comment on attachment 558767 [details] [diff] [review]
patch v3

There's a stray semicolon in my patch. I wonder why I never noticed it? :(

Comment 9

[Geoff Lankow (:darktrojan)]

Attachment: patch v4

Try again, shall we?

Comment 10

[Shawn Wilsher :sdwilsh]

Comment on attachment 559914 [details] [diff] [review]
patch v4

r=sdwilsh

Comment 11

[Ed Morley [:emorley]]

This is now in my queue of things that are being sent to try then onto inbound :-)

Comment 12

[Ed Morley [:emorley]]

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=f07268905b26

https://hg.mozilla.org/integration/mozilla-inbound/rev/ebbc50bbebeb

Comment 13

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/ebbc50bbebeb

Comment 14

[Eric Shepherd [:sheppy]]

Documented:

https://developer.mozilla.org/en/JavaScript_code_modules/FileUtils.jsm#The_File_constructor

And mentioned on Firefox 9 for developers