Closed
Bug 148918
Opened 22 years ago
Closed 22 years ago
nsILocalFile.idl clean up
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: dougt, Assigned: dougt)
Details
Attachments
(1 file)
7.63 KB,
patch
|
darin.moz
:
review+
shaver
:
superreview+
dbaron
:
approval+
|
Details | Diff | Splinter Review |
Move the public C function to nsXPCOM.h
Assignee | ||
Comment 1•22 years ago
|
||
cleans up the file interfaces and clarifies the charset of the ACString parameters
Assignee | ||
Updated•22 years ago
|
Summary: nsILocalFile.idl clean up → nsILocalFile.idl clean up
Comment 2•22 years ago
|
||
Comment on attachment 86164 [details] [diff] [review] patch v.1 so the intent is to move all XPCOM extern "C" entry points into nsXPCOM.h?? works for me i guess. r/sr=darin
Attachment #86164 -
Flags: review+
Assignee | ||
Comment 3•22 years ago
|
||
thanks for the review. I am not moving all entry points into nsXPCOM.h, just ones that are part of my notion of core xpcom.
Comment 4•22 years ago
|
||
I'm fine with the comment changes, but why should all NS_New*LocalFile extern declarations be moved into one master .h file? That just makes for excessive function-wise dependencies and recompiles, I say. Or does every file in the system include nsXPCOM.h already? Cc'ing other xpcom peers for comments. /be
Assignee | ||
Comment 5•22 years ago
|
||
We have been promoting nsXPCOM.h as the top level include for core xpcom. This files is #included by nsIServiceManager.idl. Most files already #include this file implictly. I am rebuilding with extensions on to check if i missed any places that don't get this file, but I would be surprized if there were more than one or two.
Comment on attachment 86164 [details] [diff] [review] patch v.1 >+/** >+ * Public Method to create an instance of a nsILocalFile. This function >+ * may be called prior to NS_InitXPCOM2. Method_s_, function_s_, etc., as the comment applies to both. Also, it's not a method, since it's not called on an object or interface pointer. How about: Public functions to create instances of nsILocalFile. These may be called prior to calling NS_InitXPCOM2, or after NS_ShutdownXPCOM has been called. (If the latter part is true, of course.) >+extern "C" NS_COM nsresult >+NS_NewLocalFile(const nsAString &path, >+ PRBool followLinks, >+ nsILocalFile* *result); >+ >+extern "C" NS_COM nsresult >+NS_NewNativeLocalFile(const nsACString &path, >+ PRBool followLinks, >+ nsILocalFile* *result); Is one-param-per-line the dominant style here? I can't be bothered to look! Feel my shame! FEEL IT! I don't really like "charset" in such documentation, instead preferring "character set" or "character encoding", but do you know what I like less? Having to re-review this patch! sr=shaver, fix the nits above.
Attachment #86164 -
Flags: superreview+
Assignee | ||
Comment 7•22 years ago
|
||
landed the fix on the trunk last night.
Comment on attachment 86164 [details] [diff] [review] patch v.1 Please land this on the 1.0.1 branch. Once there, remove the "mozilla1.0.1+" keyword, and add the "fixed1.0.1"
Attachment #86164 -
Flags: approval+
Keywords: mozilla1.0.1 → mozilla1.0.1+
Assignee | ||
Updated•22 years ago
|
Keywords: fixed1.0.1
Updated•22 years ago
|
Keywords: mozilla1.0.1+
You need to log in
before you can comment on or make changes to this bug.
Description
•