Created attachment 119232 [details] Patch

Comment on attachment 119232 [details] Patch I'm not sure that this makes sense as a global setting. Some scripts may be harmonious, others not, so we're going to need harmony to be enabled on a source-by-source basis, not as a global option. I don't think a 'harmony' flag in the JSC framework is needed. The command shell may want to look for this flag as an instruction as to how the source it reads in should be interpreted - to create SourceCode/SourceProvider objects with an ES6 flag set (perhaps this should be a new default-to-false argument to makeSource). I don't know what you'd do with a harmonyBlockScoping global setting - for ES5 scripts it only makes sense to not use harmony scoping rules, for ES6 scripts it only makes sense to do so. We generally don't accept changes that are not complete in their own right, and are effectively dead code - which this is. Whilst we do like to see changes broken down into smaller patches where sensible, I'd suggest that this is a small enough change that it may be wiser to introduce this alongside the first actual harmony feature implemented.

(In reply to comment #2) > (From update of attachment 119232 [details]) >The command shell may want to look for this flag as an instruction as to how the source it reads in should be interpreted - to create SourceCode/SourceProvider objects with an ES6 flag set (perhaps this should be a new default-to-false argument to makeSource). This can make sense, yes. > I don't know what you'd do with a harmonyBlockScoping global setting - for ES5 scripts it only makes sense to not use harmony scoping rules, for ES6 scripts it only makes sense to do so. Block scoping is actually a compatible change, as far as strict-mode ES5 goes, so it can make sense to say "ES5 strict-mode also has the compatible parts of ES6" -- the "compatible extended mode" I mentioned in https://lists.webkit.org/pipermail/webkit-dev/2011-December/018903.html. > I'd suggest that this is a small enough change that it may be wiser to introduce this alongside the first actual harmony feature implemented. Fair enough. Thanks for the review :)