Closed Bug 780501 Opened 8 years ago Closed 8 years ago

[OS.File] Path types

Categories

(Toolkit :: OS.File, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: Yoric, Assigned: Yoric)

References

Details

(Keywords: dev-doc-needed)

Attachments

(1 file, 2 obsolete files)

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.
Attached patch Path types (obsolete) — Splinter Review
Assignee: nobody → dteller
Attachment #649124 - Flags: review?(nfroyd)
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?
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 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.
Attachment #649124 - Flags: review?(nfroyd) → review+
Attached patch Path types (obsolete) — Splinter Review
Attachment #649124 - Attachment is obsolete: true
Attachment #652359 - Flags: review+
Attached patch Path typesSplinter Review
Attachment #652359 - Attachment is obsolete: true
Attachment #652360 - Flags: review+
Not really.
Flags: in-testsuite? → in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/277e7c231f1a
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.