958119 - Fixup style nits from Bug 939294

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Tom S [:evilpie]]

I am going to fix those as soon as possible.

Comment 1

[Gabor Krizsanits (INACTIVE)]

I talked to Bobby about this, and we agree that it's enough if you fix the isPrimitive/isObject part and just use whatever style the actual function signature uses (like you do). Also if you really don't have time for this for whatever reason just assign it to me, I don't want to hold you up with this too much.

Comment 2

[Tom S [:evilpie]]

So wait about that function signature stuff. From what I understand XPConnect at least right now, still uses JS-style, which is JSObject *foo.

Comment 3

[Gabor Krizsanits (INACTIVE)]

(In reply to Tom Schuster [:evilpie] from comment #2)
> So wait about that function signature stuff. From what I understand
> XPConnect at least right now, still uses JS-style, which is JSObject *foo.

Well... theoretically yes it should always use JS-style (so Class *foo etc.). But in reality a bunch of old files using all kind of styles sometimes even mixed in the same function, or even function signature. It would be great to clean up all this mess, little by little, but as Bobby pointed it out we should wait until the style related discussions settle somewhere, and there might be someone kind enough to write a script that fixes all this.

It also probably gives little benefit if we fix the signatures like I suggested, and then in the function body, other style is used. So if it makes sense in the given context that you're editing to enforce JS-style, please do that. But it requires to much changes, or the whole file/function is using different style than just stick to that. If it's possible don't mix too many styles at least... It's quite a mess, so I don't think it makes sense for you to put too much effort into this for now.

Comment 4

[Tom S [:evilpie]]

Attachment: fix some

I fixed most of the stuff that was obvious and some other random stuff. I didn't however feel like reformatting most of XPConnect in this patch ;)

Comment 5

[Gabor Krizsanits (INACTIVE)]

Comment on attachment 8361908 [details] [diff] [review]
fix some

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

Holy moly Tom!

(In reply to Tom Schuster [:evilpie] from comment #4)
> I fixed most of the stuff that was obvious and some other random stuff. I
> didn't however feel like reformatting most of XPConnect in this patch ;)

Ah, yeah... only half of XPConnect I see ;) Thanks a lot for this, this is an awesome
cleanup patch. I have a few nits, but even without those fixed I very r+ this patch.

 NS_IMETHODIMP
 mozJSComponentLoader::ImportInto(const nsACString & aLocation,
                                  JSObject *aTargetObj,
-                                 nsAXPCNativeCallContext * cc,
+                                 nsAXPCNativeCallContext *cc,
                                  JSObject **_retval)

const nsACString &aLocation


      nsresult DoLoadSubScriptWithOptions(const nsAString& url,
                                         LoadSubScriptOptions& options,
-                                        JSContext* cx,
+                                        JSContext *cx,
                                         JS::MutableHandle<JS::Value> retval);

& and * should follow the same rule.

Comment 6

[Tom S [:evilpie]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/888e98b2369a

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/888e98b2369a