Closed
Bug 641254
Opened 13 years ago
Closed 13 years ago
file.open() and text-streams/byte-streams docs are confusing
Categories
(Add-on SDK Graveyard :: Documentation, defect)
Add-on SDK Graveyard
Documentation
Tracking
(Not tracked)
RESOLVED
FIXED
1.0b4
People
(Reporter: asqueella, Assigned: adw)
Details
Attachments
(1 file)
1.79 KB,
patch
|
wbamberg
:
review+
|
Details | Diff | Splinter Review |
https://github.com/mozilla/addon-sdk/blob/master/packages/api-utils/docs/file.md says: > @returns {stream} A stream that can be used to access or modify > the contents of the file. See text-streams and byte-streams for > more information. while the linked text-streams https://github.com/mozilla/addon-sdk/blob/master/packages/api-utils/docs/text-streams.md says: > The text-streams module provides streams for reading and writing text and then goes on to describe two classes TextReader/TextWriter, which are documented to take an 'inputStream' in their constructor. The fact that a module claiming to provide "streams" only has constructors taking streams as parameters is interesting, but I didn't pay much attention to it. The logical thing to do seemed to call TextWriter(file.open(.., "w")), since file.open() returns a stream, and TextWriter takes a stream. That fails in very non-obvious ways: - Error: The stream is closed and cannot be used.' when calling method: [nsIOutputStream::close]" nsresult: "0x8057001c (NS_ERROR_XPC_JS_THREW_JS_OBJECT - Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIBufferedOutputStream.close]" - [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIBufferedOutputStream.flush]" I think that: - file docs should specify the exact types of objects that are returned (TextReader/TextWriter/ByteReader/ByteWriter) and not call them streams. - text-streams / byte-streams should probably not call themselves streams to avoid confusion with nsI streams. The docs should say that other APIs usually return the Text/ByteReader/Writer object and not the low-level (nsI...) streams.
Reporter | ||
Comment 1•13 years ago
|
||
Apparently I'm not alone in interpreting docs like this: http://groups.google.com/group/mozilla-labs-jetpack/browse_thread/thread/92a0de7a8aca05e9
Assignee | ||
Comment 2•13 years ago
|
||
I think this is less confusing than you and the other guy make it out to be. The text-streams and byte-streams docs specifically say their constructors take nsI streams. There's no handwaving about it. I agree that file.open() should describe the types of objects it returns though. That's what this patch does. (In reply to comment #0) > The fact that a module claiming to provide "streams" only has constructors > taking streams as parameters is interesting, but I didn't pay much attention to > it. No, not really. Streams encapsulating and consuming other streams is pretty commonplace. Look no further than nsIBufferedInputStream, nsIBinaryOutputStream and other Gecko stream interfaces.
Attachment #519590 -
Flags: review?(wbamberg)
Reporter | ||
Comment 3•13 years ago
|
||
Thanks, specifying the exact type of the object returned does make this much less confusing!
Comment 4•13 years ago
|
||
Comment on attachment 519590 [details] [diff] [review] patch Thanks Drew. That makes sense to me.
Attachment #519590 -
Flags: review?(wbamberg) → review+
Assignee | ||
Comment 5•13 years ago
|
||
https://github.com/mozilla/addon-sdk/commit/36af513a23cd6796e5c50fabc3c190829919e9ae
Assignee: nobody → adw
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0b4
You need to log in
before you can comment on or make changes to this bug.
Description
•