1090695 - Reduce indent in Parser<ParseHandler>::expr.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tooru Fujisawa [:arai]]

Derived from bug 1066827 comment #43

http://hg.mozilla.org/mozilla-central/file/tip/js/src/frontend/Parser.cpp#l5746
>    if (matched) {
>        ....
>        return seq
>    }
>    return pn;

Invert above condition and reduce indent.

Comment 1

[Tooru Fujisawa [:arai]]

I'm going to label this as [good first bug].  Required code change should be simple enough:
  * Reduce the indent of then-clause of "if (matched)" in Parser<ParseHandler>::expr method [1].
    Then-clause always returns, and code after the if is only "return pn;",
    so inverting the condition will reduce the entire indent.

You'll need basic knowledge of C++, and be able to build and test Firefox [2].

A skilled first-time contributor should be able to finish this in two weeks.  Leave comments / questions here, or ask me (:arai) in IRC #introduction and #jsapi channels.
Note that I'm not a module peer, so I'll finally forward review request to appropriate reviewer :)

[1] http://hg.mozilla.org/mozilla-central/file/a1e66a69688f/js/src/frontend/Parser.cpp#l6296
(note that I linked to specific revision of the file to point the method, and it may be not latest)
[2] https://developer.mozilla.org/en-US/docs/Simple_Firefox_build

Comment 2

[Xi Yang]

Attachment: bug_1090695.patch

Comment 3

[Tooru Fujisawa [:arai]]

Comment on attachment 8639003 [details] [diff] [review]
bug_1090695.patch

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

Thank you for your work :)
The code change looks perfect!

It would be nice if you can provide your name, e-mail, and commit message in your patch file, so the patch file become ready for checkin ;)
If you're using mercurial, try using `hg export` command, after setting the commit message (Bug 1090695 - Reduce indent in Parser<ParseHandler>::expr.) and configuring your .hgrc to generate a nice diff, by adding following 2 sections:


[ui]
username = your name <your e-mail address>

[diff]
git = 1
showfunc = 1
unified = 8


For more information, please refer following page (please ignore the "obsolete" mark on the page, we're going to make a new documentation)
https://developer.mozilla.org/en-US/docs/Archive/Mozilla/Mercurial/Using_Mercurial#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F


Then, do you have try server access to test this patch?  if not, let me know (via bug comment or on IRC :arai), I'll push it there and post the URL for the test result here.

Comment 4

[Xi Yang]

Attachment: bug_1090695.patch

Hi,

I've made the changes accordingly. Could you please help me to push to testing server? Thanks! :)

Comment 5

[Tooru Fujisawa [:arai]]

Excellent! :D

Here's try push, it will take 2 hours or so.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bce37a47554e

Comment 6

[Tooru Fujisawa [:arai]]

Comment on attachment 8639309 [details] [diff] [review]
bug_1090695.patch

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

All green on try run :)
It's ready for check in.

Thanks again for your patch!

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/26d59ea97d2d

Comment 8

[Xi Yang]

You are welcome. btw, is there anything else I can help with?

(In reply to Tooru Fujisawa [:arai] from comment #6)
> Comment on attachment 8639309 [details] [diff] [review]
> bug_1090695.patch
> 
> Review of attachment 8639309 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> All green on try run :)
> It's ready for check in.
> 
> Thanks again for your patch!

Comment 9

[Tooru Fujisawa [:arai]]

(In reply to Xi Yang from comment #8)
> You are welcome. btw, is there anything else I can help with?

Yeah, there're a lot of things to do.  Have you checked other bugs in Bugs Ahoy?
  http://www.joshmatthews.net/bugsahoy/

If you're interested in JavaScript Engine, choose it in sidebar, there mentored bugs like this will be shown.  Besides, searching unassigned bugs on bugzilla would also be a nice way to find a bug to work on.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/26d59ea97d2d