Convert Settings.in, CSSValueKeywords.in, SVGCSSValueKeywords.in to json5

Convert Settings.in, CSSValueKeywords.in, SVGCSSValueKeywords.in to json5

Add json5_generator similar to in_generator and in_file to read json5
configs with default and valid_value&valid_type support.

Link to design doc in bug.

BUG=677884
Review-Url: https://codereview.chromium.org/2620883002
Cr-Commit-Position: refs/heads/master@{#443949}
Committed: https://chromium.googlesource.com/chromium/src/+/4a9064a99588486772c9c2ec88e84e2a4f444f65

[ktyliu, 2017-01-12 05:22:44]

The CQ bit was checked by ktyliu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 05:23:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ktyliu, 2017-01-12 05:24:56]

Description was changed from

==========
Convert .in to .json5

BUG=
==========

to

==========
Convert Settings.in, CSSValueKeywords.in, SVGCSSValueKeywords.in to json5

Add json5_generator similar to in_generator and in_file to read json5
configs with default and valid_value&valid_type support.

BUG=677884
==========

[ktyliu, 2017-01-12 05:26:57]

ktyliu@chromium.org changed reviewers:
+ alancutter@chromium.org, sashab@chromium.org

[ktyliu, 2017-01-12 05:26:58]

Hi Sasha, Alan,

Looks like we've agreed to use JSON5 format for our configuration files.

This CL updates three .in files as examples. Can you check this looks okay to
you?

Any comments / suggestions welcome.

Thanks,
Kevin

[sashab, 2017-01-12 05:33:47]

sashab@chromium.org changed reviewers:
+ dcheng@chromium.org, dpranke@chromium.org

[sashab, 2017-01-12 05:33:47]

LGTM from me, excellent comment in json5_generator.py.

Before we land I'd like a review from dpranke@ and also from dcheng@ who will
have some feedback from a code style perspective.

Please mention in CL description "Link to design doc available in bug".

Well done!

[ktyliu, 2017-01-12 05:35:52]

Description was changed from

==========
Convert Settings.in, CSSValueKeywords.in, SVGCSSValueKeywords.in to json5

Add json5_generator similar to in_generator and in_file to read json5
configs with default and valid_value&valid_type support.

BUG=677884
==========

to

==========
Convert Settings.in, CSSValueKeywords.in, SVGCSSValueKeywords.in to json5

Add json5_generator similar to in_generator and in_file to read json5
configs with default and valid_value&valid_type support.

Link to design doc in bug.

BUG=677884
==========

[commit-bot: I haz the power, 2017-01-12 05:58:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-12 05:58:14]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[ktyliu, 2017-01-12 06:07:20]

The CQ bit was checked by ktyliu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 06:07:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ktyliu, 2017-01-12 06:08:43]

Done.
Thanks for the review and guidance in the project.

Looks like test failed because SettingsMacros.h.tmpl wasn't precise with initial
testing. Fixed that too.

[commit-bot: I haz the power, 2017-01-12 08:00:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-12 08:00:38]

Dry run: This issue passed the CQ dry run.

[dcheng, 2017-01-16 05:52:52]

It doesn't seem like this really changes code style, so from that perspective,
lgtm. Let me know if I'm missing something...

https://codereview.chromium.org/2620883002/diff/90001/third_party/WebKit/Sour...
File third_party/WebKit/Source/build/scripts/json5_generator.py (right):

https://codereview.chromium.org/2620883002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/json5_generator.py:225: parser =
optparse.OptionParser()
Nit: use argparse, optparse is deprecated in 2.7

https://codereview.chromium.org/2620883002/diff/90001/third_party/WebKit/Sour...
File third_party/WebKit/Source/build/scripts/name_utilities.py (right):

https://codereview.chromium.org/2620883002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/name_utilities.py:101: return value
I'm a bit surprised that this isn't just:

return 'true' if value else false

It'd be nice to document why it needs to return the original value as-is.

[ktyliu, 2017-01-16 06:19:55]

ktyliu@chromium.org changed reviewers:
- alancutter@chromium.org, dpranke@chromium.org

[ktyliu, 2017-01-16 06:19:56]

Thanks, Daniel, for the review and suggestions. All done.

Yes this doesn't change the generated C++ code but just the config file format
and small tweak to the template.

https://codereview.chromium.org/2620883002/diff/90001/third_party/WebKit/Sour...
File third_party/WebKit/Source/build/scripts/json5_generator.py (right):

https://codereview.chromium.org/2620883002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/json5_generator.py:225: parser =
optparse.OptionParser()
On 2017/01/16 at 05:52:52, dcheng wrote:
> Nit: use argparse, optparse is deprecated in 2.7

Thanks for pointing it out.
Changed to use argparse and removed (sys.)argv

https://codereview.chromium.org/2620883002/diff/90001/third_party/WebKit/Sour...
File third_party/WebKit/Source/build/scripts/name_utilities.py (right):

https://codereview.chromium.org/2620883002/diff/90001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/name_utilities.py:101: return value
On 2017/01/16 at 05:52:52, dcheng wrote:
> I'm a bit surprised that this isn't just:
> 
> return 'true' if value else false
> 
> It'd be nice to document why it needs to return the original value as-is.
Added comment pointing out it can be a platform-dependent const such as
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/Set...

[ktyliu, 2017-01-16 06:21:34]

The CQ bit was checked by ktyliu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-16 06:21:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-16 07:53:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-16 07:53:26]

Dry run: This issue passed the CQ dry run.

[ktyliu, 2017-01-16 21:53:30]

The CQ bit was checked by ktyliu@chromium.org

[ktyliu, 2017-01-16 21:53:31]

The patchset sent to the CQ was uploaded after l-g-t-m from sashab@chromium.org,
dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2620883002/#ps110001
(title: "Use argparse")

[commit-bot: I haz the power, 2017-01-16 21:53:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-16 21:58:02]

CQ is committing da patch.

Bot data: {"patchset_id": 110001, "attempt_start_ts": 1484603610622790,
"parent_rev": "8dffb3c9d60eabf71c44980fbbab1d61d7e9640f", "commit_rev":
"4a9064a99588486772c9c2ec88e84e2a4f444f65"}

[commit-bot: I haz the power, 2017-01-16 21:58:36]

Description was changed from

==========
Convert Settings.in, CSSValueKeywords.in, SVGCSSValueKeywords.in to json5

Add json5_generator similar to in_generator and in_file to read json5
configs with default and valid_value&valid_type support.

Link to design doc in bug.

BUG=677884
==========

to

==========
Convert Settings.in, CSSValueKeywords.in, SVGCSSValueKeywords.in to json5

Add json5_generator similar to in_generator and in_file to read json5
configs with default and valid_value&valid_type support.

Link to design doc in bug.

BUG=677884

Review-Url: https://codereview.chromium.org/2620883002
Cr-Commit-Position: refs/heads/master@{#443949}
Committed:
https://chromium.googlesource.com/chromium/src/+/4a9064a99588486772c9c2ec88e8...
==========

[commit-bot: I haz the power, 2017-01-16 21:58:38]

Committed patchset #7 (id:110001) as
https://chromium.googlesource.com/chromium/src/+/4a9064a99588486772c9c2ec88e8...