148918 - nsILocalFile.idl clean up

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Doug Turner (:dougt)]

Move the public C function to nsXPCOM.h

Comment 1

[Doug Turner (:dougt)]

Attachment: patch v.1

cleans up the file interfaces and clarifies the charset of the ACString
parameters

Comment 2

[Darin Fisher]

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

Comment 3

[Doug Turner (:dougt)]

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

[Brendan Eich [:brendan]]

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

Comment 5

[Doug Turner (:dougt)]

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 6

[Mike Shaver (:shaver emeritus)]

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.

Comment 7

[Doug Turner (:dougt)]

landed the fix on the trunk last night.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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"