Closed Bug 699041 Opened 8 years ago Closed 8 years ago

Remove MBool / MB_TRUE / MB_FALSE

Categories

(Core :: XSLT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: Ms2ger, Assigned: abhishekkumarsingh.cse)

Details

(Whiteboard: [good first bug][mentor=Ms2ger])

Attachments

(1 file, 3 obsolete files)

They can simply be replaced with bool/true/false.
Assignee: nobody → abhishekkumarsingh.cse
Status: NEW → ASSIGNED
Attached patch Patch_v (obsolete) — Splinter Review
Attachment #572440 - Flags: review?(Ms2ger)
Comment on attachment 572440 [details] [diff] [review]
Patch_v

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

it seems like there are some big whitespace issues in this patch. Looks good otherwise though, but that needs to be fixed before this can land.

::: content/xslt/src/base/txCore.h
@@ +108,5 @@
>  
>  // XXX These should go away eventually.
>  #define TxObject txObject
>  typedef txDouble Double;
> +typedef boolbool;

This line looks all wrong

@@ +113,3 @@
>  
> +
> +

And just remove these lines rather than adding to empty lines.

::: content/xslt/src/base/txExpandedNameMap.h
@@ +94,5 @@
>                mCurrentPos(PRUint32(-1))
>          {
>          }
>  
> +       bool next()

Whitespace here is wrong.

::: content/xslt/src/base/txList.cpp
@@ +281,3 @@
>  **/
> +bool txListIterator::hasNext() {
> +   bool hasNext = false;

And here.

@@ +296,3 @@
>  **/
> +bool txListIterator::hasPrevious() {
> +   bool hasPrevious = false;

And here :(
Attachment #572440 - Flags: review?(Ms2ger) → review-
Attached patch Patch_v1 (obsolete) — Splinter Review
Attachment #572745 - Flags: review?(Ms2ger)
Comment on attachment 572745 [details] [diff] [review]
Patch_v1

>--- a/content/xslt/src/base/txCore.h	Sat Nov 05 23:41:16 2011 +0200
>+++ b/content/xslt/src/base/txCore.h	Tue Nov 08 17:52:26 2011 +0530
>-typedef bool MBool;
>+typedef bool bool;

Remove this line

>--- a/content/xslt/src/base/txList.h	Sat Nov 05 23:41:16 2011 +0200
>+++ b/content/xslt/src/base/txList.h	Tue Nov 08 17:52:26 2011 +0530

Indentation is off in this file.

>--- a/content/xslt/src/xml/txDOM.h	Sat Nov 05 23:41:16 2011 +0200
>+++ b/content/xslt/src/xml/txDOM.h	Tue Nov 08 17:52:26 2011 +0530
>-    virtual MBool hasChildNodes() const = 0;
>+    virtualbool hasChildNodes() const = 0;

And this won't work.

>-    MBool hasChildNodes() const;
>+   bool hasChildNodes() const;

Indentation.

Same comments elsewhere in the patch.
Attachment #572745 - Flags: review?(Ms2ger) → review-
It appears that you've replaced " MBool" with "bool" in both versions of this patch which is leading to the whitespace issue. Please just replace "MBool" with "bool" in order to fix it. This is happening everywhere, not just in the places commented on so far.
Attached patch Patch_A (obsolete) — Splinter Review
Attachment #573170 - Flags: review?(Ms2ger)
Comment on attachment 573170 [details] [diff] [review]
Patch_A

>--- a/content/xslt/src/base/txCore.h	Mon Nov 07 07:16:24 2011 -0500
>+++ b/content/xslt/src/base/txCore.h	Thu Nov 10 00:24:50 2011 +0530
>@@ -104,15 +104,13 @@ public:
>      * represent a double, NaN will be returned.
>      */
>     static double toDouble(const nsAString& aStr);
> };
> 
> // XXX These should go away eventually.
> #define TxObject txObject
> typedef txDouble Double;
>-typedef bool MBool;
>+typedef bool bool;

This typedef still needs to go. Looks good otherwise.

I can't review this, though. Jonas?
Attachment #573170 - Flags: review?(jonas)
Attachment #573170 - Flags: review?(Ms2ger)
Attachment #573170 - Flags: feedback+
Comment on attachment 573170 [details] [diff] [review]
Patch_A

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

r=me with that fixed.

::: content/xslt/src/base/txCore.h
@@ +108,5 @@
>  
>  // XXX These should go away eventually.
>  #define TxObject txObject
>  typedef txDouble Double;
> +typedef bool bool;

Remove this
Attachment #573170 - Flags: review?(jonas) → review+
Abhishek, if you could generate patch with that line removed, and as described at <https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F>, that would be really helpful.

Thanks for taking this bug!
Attached patch Patch_A1Splinter Review
Attachment #573170 - Attachment is obsolete: true
Keywords: checkin-needed
patch failed to apply in content/xslt/src/xpath/txExprParser.cpp
Keywords: checkin-needed
Attachment #572745 - Attachment is obsolete: true
Attachment #572440 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/33d34da275ed
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in before you can comment on or make changes to this bug.