Issue 4353 - Remove non standard for each syntax

Issue 4353 - Remove non standard for each syntax

[kzar, 2016-08-24 11:10:02]

Patch Set 1

[Sebastian Noack, 2016-08-24 11:34:13]

LGTM

[Wladimir Palant, 2016-08-24 14:21:51]

I assume that you compared the output of the original version (and old JS Shell)
with the new one? There are always differences, and we should make sure that
these aren't relevant to our code.

https://codereview.adblockplus.org/29350140/diff/29350141/scripts/astDecompil...
File scripts/astDecompile.js (right):

https://codereview.adblockplus.org/29350140/diff/29350141/scripts/astDecompil...
scripts/astDecompile.js:449: return "<>" +  
ast.contents.map(decompileAST).join("") + "</>";
Nit: You've added unnecessary whitespace here.

https://codereview.adblockplus.org/29350140/diff/29350141/scripts/astDecompil...
scripts/astDecompile.js:499: }
It's probably better to just remove all functions related to XML literals -
these are no longer supported by SpiderMonkey (yes,
https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Parser...
appears to be outdated in this respect). So everything starting with `/* E4X */`
until this function can go.

https://codereview.adblockplus.org/29350140/diff/29350141/scripts/decompile.js
File scripts/decompile.js (right):

https://codereview.adblockplus.org/29350140/diff/29350141/scripts/decompile.j...
scripts/decompile.js:1: // Decompile a JS file. This will be painful.
I don't consider it a good measure to modify files that we don't use and
consequently cannot really test. Everything modified here other than
astDecompile.js is dead code now, it should be better to remove it.

[kzar, 2016-08-24 14:43:21]

Patch Set 2 : Removed unused code

Yep, I ran `./build.py -t chrome devenv` before and after these changes. Only
difference in genreated output was the devenvVersion__ file. (I also ran `python
autotest.py` and those still pass.)

https://codereview.adblockplus.org/29350140/diff/29350141/scripts/astDecompil...
File scripts/astDecompile.js (right):

https://codereview.adblockplus.org/29350140/diff/29350141/scripts/astDecompil...
scripts/astDecompile.js:449: return "<>" +  
ast.contents.map(decompileAST).join("") + "</>";
On 2016/08/24 14:21:50, Wladimir Palant wrote:
> Nit: You've added unnecessary whitespace here.

Done.

https://codereview.adblockplus.org/29350140/diff/29350141/scripts/astDecompil...
scripts/astDecompile.js:499: }
On 2016/08/24 14:21:50, Wladimir Palant wrote:
> It's probably better to just remove all functions related to XML literals -
> these are no longer supported by SpiderMonkey (yes,
>
https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Parser...
> appears to be outdated in this respect). So everything starting with `/* E4X
*/`
> until this function can go.

Done.

https://codereview.adblockplus.org/29350140/diff/29350141/scripts/decompile.js
File scripts/decompile.js (right):

https://codereview.adblockplus.org/29350140/diff/29350141/scripts/decompile.j...
scripts/decompile.js:1: // Decompile a JS file. This will be painful.
On 2016/08/24 14:21:50, Wladimir Palant wrote:
> I don't consider it a good measure to modify files that we don't use and
> consequently cannot really test. Everything modified here other than
> astDecompile.js is dead code now, it should be better to remove it.

Done.

[Wladimir Palant, 2016-08-24 14:47:53]

LGTM