780501 - [OS.File] Path types

Status: RESOLVED FIXED

(Toolkit Graveyard :: OS.File, enhancement)

Description

[David Teller [:Yoric] - still alive but not very active]

To simplify the work of serialization in bug 777711, we should define the a type |path|, defined as a C string under Unix and a wide string under Windows.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

Attachment: Path types

Comment 2

[Nathan Froyd [:froydnj]]

These changes are purely so we can attach a special serializer to the type, correct?  What's not to like about the current string serialization?

Comment 3

[David Teller [:Yoric] - still alive but not very active]

I want to use the serialization defined in OS.Shared and I prefer writing the following:

 Type.path.toMsg(...)

to the following:

 if (/*determine that we are under Windows*/) {
   Type.wstring.toMsg(...)
 } else {
   Type.cstring.toMsg(...)
 }

The rest of the patch is a cosmetic change, mostly for documentation purposes (although there could be applications if we ever want to care about file system name encodings).

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 649124 [details] [diff] [review]
Path types

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

Ah, yes, of course.  Can I blame Monday mornings for that one?

::: toolkit/components/osfile/osfile_win_back.jsm
@@ +206,5 @@
>  
>         WinFile.CopyFile =
>           declareFFI("CopyFileW", ctypes.winapi_abi,
>                      /*return*/ Types.zero_or_nothing,
> +                    /*sourcePath*/ Types.path,

These sorts of things are nice cleanups.

Comment 5

[David Teller [:Yoric] - still alive but not very active]

Attachment: Path types

Comment 6

[David Teller [:Yoric] - still alive but not very active]

Attachment: Path types

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Green on Try.
https://tbpl.mozilla.org/?tree=Try&rev=2ec9ffb953d7

https://hg.mozilla.org/integration/mozilla-inbound/rev/277e7c231f1a

Should this have tests?

Comment 8

[David Teller [:Yoric] - still alive but not very active]

Not really.

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/277e7c231f1a