Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(54)

Issue 2645283006: Convert make_qualified_names and make_element_factory to use JSON5. (Closed)

Created:
3 years, 11 months ago by ktyliu
Modified:
3 years, 10 months ago
Reviewers:
nainar, sashab
CC:
blink-reviews, blink-reviews-html_chromium.org, chromium-reviews, dglazkov+blink, dominicc+watchlist_chromium.org, krit, f(malita), fs, gyuyoung2, kinuko+watch, kouhei+svg_chromium.org, loading-reviews+parser_chromium.org, pdr+svgwatchlist_chromium.org, rwlbuis, Stephen Chennney
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Convert make_qualified_names and make_element_factory to use JSON5. Convert make_qualified_names.py, make_element_factory.py, make_element_type_helpers.py, and make_element_lookup_trie.py to use JSON5 config format. Converted these config files accordingly: Source/core/html/HTMLAttributeNames.in Source/core/html/HTMLTagNames.in Source/core/html/parser/MathMLAttributeNames.in Source/core/html/parser/MathMLTagNames.in Source/core/svg/SVGAttributeNames.in Source/core/svg/SVGTagNames.in Source/core/svg/xlinkattrs.in Source/core/xml/xmlattrs.in Source/core/xml/xmlnsattrs.in Note that these files are updated together because: 1. make_element_factory.py depends on make_qualified_names.py 2. HTMLTagNames.in is common input to the scripts make_element_factory, make_element_type_helpers and make_element_lookup_trie Removed FIXME in make_element_type_helpers.py since that case no longer occurs. Also fixed bug in json5_generator where parameters should not be validated if default_parameters is not provided. (caught when I had typo on default_parameters in the script) BUG=677884 Review-Url: https://codereview.chromium.org/2645283006 Cr-Original-Original-Original-Commit-Position: refs/heads/master@{#447450} Committed: https://chromium.googlesource.com/chromium/src/+/f5ca5654596eeda38c2a796bbbad299d92b7f008 Review-Url: https://codereview.chromium.org/2645283006 Cr-Original-Original-Commit-Position: refs/heads/master@{#447891} Committed: https://chromium.googlesource.com/chromium/src/+/701ccf95a0ec04657b59ad240e28be4085fe033f Review-Url: https://codereview.chromium.org/2645283006 Cr-Original-Commit-Position: refs/heads/master@{#449229} Committed: https://chromium.googlesource.com/chromium/src/+/013b234ed4fc6fd9a999bc2bd5f156a17c810356 Review-Url: https://codereview.chromium.org/2645283006 Cr-Commit-Position: refs/heads/master@{#449371} Committed: https://chromium.googlesource.com/chromium/src/+/5df01277cc419f8b19d266412738704022cda5d3

Patch Set 1 #

Patch Set 2 : Fix parameters skip #

Total comments: 4

Patch Set 3 : rebase and address reviewer comments #

Patch Set 4 : remove \\ #

Patch Set 5 : Rebase -- add aria-modal to HTMLAttributeNames #

Patch Set 6 : Rebase #

Patch Set 7 : Rebase -- add aria-modal to HTMLAttributeNames #

Patch Set 8 : fix typo __majson5__ -> __main__ in make_qualified_names.py #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1359 lines, -940 lines) Patch
M third_party/WebKit/Source/build/scripts/json5_generator.py View 1 2 3 4 1 chunk +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/build/scripts/make_element_factory.py View 4 chunks +16 lines, -16 lines 0 comments Download
M third_party/WebKit/Source/build/scripts/make_element_lookup_trie.py View 3 chunks +14 lines, -14 lines 0 comments Download
M third_party/WebKit/Source/build/scripts/make_element_type_helpers.py View 5 chunks +20 lines, -24 lines 0 comments Download
M third_party/WebKit/Source/build/scripts/make_qualified_names.py View 1 2 3 4 5 6 7 3 chunks +25 lines, -28 lines 0 comments Download
M third_party/WebKit/Source/core/BUILD.gn View 1 2 3 4 5 9 chunks +12 lines, -12 lines 0 comments Download
D third_party/WebKit/Source/core/html/HTMLAttributeNames.in View 1 2 3 4 1 chunk +0 lines, -348 lines 0 comments Download
A third_party/WebKit/Source/core/html/HTMLAttributeNames.json5 View 1 2 3 4 1 chunk +354 lines, -0 lines 0 comments Download
D third_party/WebKit/Source/core/html/HTMLTagNames.in View 1 chunk +0 lines, -145 lines 0 comments Download
A third_party/WebKit/Source/core/html/HTMLTagNames.json5 View 1 2 3 1 chunk +481 lines, -0 lines 0 comments Download
D third_party/WebKit/Source/core/html/parser/MathMLAttributeNames.in View 1 chunk +0 lines, -6 lines 0 comments Download
A third_party/WebKit/Source/core/html/parser/MathMLAttributeNames.json5 View 1 2 3 1 chunk +12 lines, -0 lines 0 comments Download
D third_party/WebKit/Source/core/html/parser/MathMLTagNames.in View 1 chunk +0 lines, -12 lines 0 comments Download
A third_party/WebKit/Source/core/html/parser/MathMLTagNames.json5 View 1 2 3 1 chunk +18 lines, -0 lines 0 comments Download
D third_party/WebKit/Source/core/svg/SVGAttributeNames.in View 1 chunk +0 lines, -243 lines 0 comments Download
A third_party/WebKit/Source/core/svg/SVGAttributeNames.json5 View 1 2 3 1 chunk +249 lines, -0 lines 0 comments Download
D third_party/WebKit/Source/core/svg/SVGTagNames.in View 1 chunk +0 lines, -71 lines 0 comments Download
A third_party/WebKit/Source/core/svg/SVGTagNames.json5 View 1 2 3 1 chunk +115 lines, -0 lines 0 comments Download
D third_party/WebKit/Source/core/svg/xlinkattrs.in View 1 chunk +0 lines, -11 lines 0 comments Download
A third_party/WebKit/Source/core/svg/xlinkattrs.json5 View 1 2 3 1 chunk +17 lines, -0 lines 0 comments Download
D third_party/WebKit/Source/core/xml/xmlattrs.in View 1 chunk +0 lines, -6 lines 0 comments Download
A third_party/WebKit/Source/core/xml/xmlattrs.json5 View 1 2 3 1 chunk +12 lines, -0 lines 0 comments Download
D third_party/WebKit/Source/core/xml/xmlnsattrs.in View 1 chunk +0 lines, -4 lines 0 comments Download
A third_party/WebKit/Source/core/xml/xmlnsattrs.json5 View 1 2 3 1 chunk +10 lines, -0 lines 0 comments Download

Messages

Total messages: 68 (43 generated)
ktyliu
Hi Jon, As Sasha wrote, we should send style project CLs to another person before ...
3 years, 11 months ago (2017-01-25 02:35:52 UTC) #6
ktyliu
Hi Naina, As Sasha wrote, we should send style project CLs to another person for ...
3 years, 11 months ago (2017-01-25 05:06:09 UTC) #12
nainar
lgtm
3 years, 10 months ago (2017-01-29 23:01:18 UTC) #15
ktyliu
Thank you, Naina, for the review! Adding Sasha for OWNERS.
3 years, 10 months ago (2017-01-29 23:02:01 UTC) #17
sashab
Nice! Great CL description and overall patch. One small question but loving the json5 files. ...
3 years, 10 months ago (2017-01-31 23:09:19 UTC) #18
ktyliu
Thanks Sasha for the thorough review. https://codereview.chromium.org/2645283006/diff/20001/third_party/WebKit/Source/build/scripts/json5_generator.py File third_party/WebKit/Source/build/scripts/json5_generator.py (right): https://codereview.chromium.org/2645283006/diff/20001/third_party/WebKit/Source/build/scripts/json5_generator.py#newcode158 third_party/WebKit/Source/build/scripts/json5_generator.py:158: entry[key] = value ...
3 years, 10 months ago (2017-01-31 23:42:06 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2645283006/40001
3 years, 10 months ago (2017-02-01 04:42:30 UTC) #26
commit-bot: I haz the power
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/f5ca5654596eeda38c2a796bbbad299d92b7f008
3 years, 10 months ago (2017-02-01 04:47:40 UTC) #29
findit-for-me
FYI: Findit identified this CL at revision 447450 as the culprit for failures in the ...
3 years, 10 months ago (2017-02-01 05:13:44 UTC) #30
tsergeant
A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/2668023003/ by tsergeant@chromium.org. ...
3 years, 10 months ago (2017-02-01 05:16:10 UTC) #31
ktyliu
On 2017/02/01 at 05:16:10, tsergeant wrote: > A revert of this CL (patchset #3 id:40001) ...
3 years, 10 months ago (2017-02-02 22:19:32 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2645283006/80001
3 years, 10 months ago (2017-02-02 23:58:36 UTC) #38
commit-bot: I haz the power
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/701ccf95a0ec04657b59ad240e28be4085fe033f
3 years, 10 months ago (2017-02-03 00:07:50 UTC) #41
ktyliu
A revert of this CL (patchset #5 id:80001) has been created in https://codereview.chromium.org/2677603002/ by ktyliu@chromium.org. ...
3 years, 10 months ago (2017-02-03 00:29:57 UTC) #42
findit-for-me
FYI: Findit identified this CL at revision 447891 as the culprit for failures in the ...
3 years, 10 months ago (2017-02-03 00:52:23 UTC) #43
ktyliu
Rebased and will try commit again chatting to Sasha.
3 years, 10 months ago (2017-02-09 03:24:11 UTC) #46
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2645283006/120001
3 years, 10 months ago (2017-02-09 03:24:45 UTC) #50
commit-bot: I haz the power
Committed patchset #7 (id:120001) as https://chromium.googlesource.com/chromium/src/+/013b234ed4fc6fd9a999bc2bd5f156a17c810356
3 years, 10 months ago (2017-02-09 06:37:41 UTC) #53
haraken
A revert of this CL (patchset #7 id:120001) has been created in https://codereview.chromium.org/2684973006/ by haraken@chromium.org. ...
3 years, 10 months ago (2017-02-09 07:26:52 UTC) #54
ktyliu
On 2017/02/09 at 07:26:52, haraken wrote: > A revert of this CL (patchset #7 id:120001) ...
3 years, 10 months ago (2017-02-09 07:29:44 UTC) #55
ktyliu
On 2017/02/09 at 07:26:52, haraken wrote: > A revert of this CL (patchset #7 id:120001) ...
3 years, 10 months ago (2017-02-09 07:29:45 UTC) #56
findit-for-me
FYI: Findit identified this CL at revision 449229 as the culprit for failures in the ...
3 years, 10 months ago (2017-02-09 07:33:29 UTC) #57
ktyliu
On 2017/02/09 at 07:33:29, findit-for-me wrote: > FYI: Findit identified this CL at revision 449229 ...
3 years, 10 months ago (2017-02-09 19:08:16 UTC) #62
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2645283006/140001
3 years, 10 months ago (2017-02-09 19:09:10 UTC) #65
commit-bot: I haz the power
3 years, 10 months ago (2017-02-09 19:19:41 UTC) #68
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/5df01277cc419f8b19d266412738...

Powered by Google App Engine
This is Rietveld 408576698