Made a generator for .h files for implementations of CSSPropertyAPI.h

Made a generator for .h files for implementations of CSSPropertyAPI.h

Renamed make_css_property_descriptor.py to make_css_property_api.py and
modified it so that it also generates API .h files. Added the template
file CSSPropertyAPI.h.tmpl.

To generate the .h file for a property, add the api_class flag to the
property in CSSProperties.in.

BUG=668012
Committed: https://crrev.com/de5be95f7da7d464a16f1a1fa930bd4340063f94
Cr-Commit-Position: refs/heads/master@{#439053}

[aazzam, 2016-12-14 23:03:21]

aazzam@google.com changed reviewers:
+ sashab@chromium.org

[aazzam, 2016-12-14 23:03:22]

I changed the generator so that it also generates .h files, please take a look
sasha :)

[sashab, 2016-12-15 02:48:43]

Great work so far! Real improvement in comments & naming which I'm glad to see.
Patch is also really well constructed, nice job Anna

https://codereview.chromium.org/2578773002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/build/scripts/make_css_property_api.py (right):

https://codereview.chromium.org/2578773002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/build/scripts/make_css_property_api.py:37: continue
Small thing - in your CL description, avoid 'we', use recipe-style language
instead 'To do X, do Y...'

https://codereview.chromium.org/2578773002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/build/scripts/make_css_property_api.py:38: classname =
get_classname(property)
Also this file - because it generates both, maybe:

make_css_property_apis.py
make_css_property_descriptors.py
make_css_property_apis_and_descriptors.py

Whatever makes more sense to you :) I think the first one is probably OK

https://codereview.chromium.org/2578773002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/build/scripts/make_css_property_api.py:40:
self._outputs.update({classname + '.h':
self.generate_property_api_h_builder(classname)})
You don't need .update - you can just set [key] = [value] :)

https://codereview.chromium.org/2578773002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/build/scripts/make_css_property_api.py:62: # which
generates a new function pointer given the classname and property ids.
Great comment, except one nit: 'function pointer' is not really the right word
in Python since pointers don't exist lol ;) I'd say function object instead.

Can probably shorten it a bit too - see if you can do that without losing
meaning :)

https://codereview.chromium.org/2578773002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/build/scripts/templates/CSSPropertyAPI.h.tmpl
(right):

https://codereview.chromium.org/2578773002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/build/scripts/templates/CSSPropertyAPI.h.tmpl:2: //
Use of this source code is governed by a BSD-style license that can be
CSSPropertyAPI.h.tmpl is probably a bit misleading since we already have a
CSSPropertyAPI.h we might want to generate later ;)

Maybe CSSPropertyAPIFiles.h.tmpl? Or CSSPropertyAPI.each.h.tmpl? We don't really
have a style for this, maybe ask around for some ideas or come up with your own
:)

https://codereview.chromium.org/2578773002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/build/scripts/templates/CSSPropertyAPI.h.tmpl:26:
#endif  // {{api_classname}}_h
Oh my god this template is so awesome... *dies*

I think this is actually *the* simplest template file we have lol

https://codereview.chromium.org/2578773002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/properties/CSSPropertyAPIWillChange.cpp
(right):

https://codereview.chromium.org/2578773002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/properties/CSSPropertyAPIWillChange.cpp:9:
#include "core/css/parser/CSSPropertyParser.h"
Oh nice! By generating the file you found a bug. Haha :D

[aazzam, 2016-12-15 04:03:15]

Description was changed from

==========
Made a generator for .h files for implementations of CSSPropertyAPI.h

Renamed make_css_property_descriptor.py to make_css_property_api.py and
modified it so that it also generates API .h files. Added the template
file CSSPropertyAPI.h.tmpl.

When we want to generate the .h file for a property, we add the
api_class flag to the property in CSSProperties.in.

BUG=668012
==========

to

==========
Made a generator for .h files for implementations of CSSPropertyAPI.h

Renamed make_css_property_descriptor.py to make_css_property_api.py and
modified it so that it also generates API .h files. Added the template
file CSSPropertyAPI.h.tmpl.

To generate the .h file for a property, add the api_class flag to the
property in CSSProperties.in.

BUG=668012
==========

[aazzam, 2016-12-15 04:35:08]

PTAL :)

https://codereview.chromium.org/2578773002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/build/scripts/make_css_property_api.py (right):

https://codereview.chromium.org/2578773002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/build/scripts/make_css_property_api.py:37: continue
On 2016/12/15 at 02:48:42, sashab wrote:
> Small thing - in your CL description, avoid 'we', use recipe-style language
instead 'To do X, do Y...'

done :)

https://codereview.chromium.org/2578773002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/build/scripts/make_css_property_api.py:38: classname =
get_classname(property)
On 2016/12/15 at 02:48:43, sashab wrote:
> Also this file - because it generates both, maybe:
> 
> make_css_property_apis.py
> make_css_property_descriptors.py
> make_css_property_apis_and_descriptors.py
> 
> Whatever makes more sense to you :) I think the first one is probably OK

I've gone with make_css_property_apis.py, sounds OK and is not too long :)

https://codereview.chromium.org/2578773002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/build/scripts/make_css_property_api.py:40:
self._outputs.update({classname + '.h':
self.generate_property_api_h_builder(classname)})
On 2016/12/15 at 02:48:43, sashab wrote:
> You don't need .update - you can just set [key] = [value] :)

done :)

https://codereview.chromium.org/2578773002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/build/scripts/make_css_property_api.py:62: # which
generates a new function pointer given the classname and property ids.
On 2016/12/15 at 02:48:42, sashab wrote:
> Great comment, except one nit: 'function pointer' is not really the right word
in Python since pointers don't exist lol ;) I'd say function object instead.
> 
> Can probably shorten it a bit too - see if you can do that without losing
meaning :)

re-worded it, does this sound better?

https://codereview.chromium.org/2578773002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/build/scripts/templates/CSSPropertyAPI.h.tmpl
(right):

https://codereview.chromium.org/2578773002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/build/scripts/templates/CSSPropertyAPI.h.tmpl:2: //
Use of this source code is governed by a BSD-style license that can be
On 2016/12/15 at 02:48:43, sashab wrote:
> CSSPropertyAPI.h.tmpl is probably a bit misleading since we already have a
CSSPropertyAPI.h we might want to generate later ;)
> 
> Maybe CSSPropertyAPIFiles.h.tmpl? Or CSSPropertyAPI.each.h.tmpl? We don't
really have a style for this, maybe ask around for some ideas or come up with
your own :)

CSSPropertyAPIFiles.h.tmpl sounds good :)

[sashab, 2016-12-15 06:15:54]

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

[sashab, 2016-12-15 06:15:54]

Looks good so far, let's get alan to take a look and I'll do another review
after :)

[alancutter (OOO until 2018), 2016-12-15 06:20:55]

lgtm

https://codereview.chromium.org/2578773002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/build/scripts/templates/CSSPropertyAPIFiles.h.tmpl
(right):

https://codereview.chromium.org/2578773002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/templates/CSSPropertyAPIFiles.h.tmpl:13:
// TODO(aazzam): Generate API .h files
Remove TODO.

[aazzam, 2016-12-15 22:12:41]

Removed the TODO

[aazzam, 2016-12-15 22:12:44]

The CQ bit was checked by aazzam@google.com

[aazzam, 2016-12-15 22:12:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
alancutter@chromium.org
Link to the patchset: https://codereview.chromium.org/2578773002/#ps40001
(title: "Removed TODO.")

[commit-bot: I haz the power, 2016-12-15 22:13:55]

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

[commit-bot: I haz the power, 2016-12-15 22:13:56]

This CL has an open dependency (Issue 2564283003 Patch 120001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[aazzam, 2016-12-15 23:17:28]

Rebased, PTAL :)

[sashab, 2016-12-15 23:22:22]

Love it!! LGTM

https://codereview.chromium.org/2578773002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/build/scripts/make_css_property_apis.py (right):

https://codereview.chromium.org/2578773002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/make_css_property_apis.py:60: # This
returned function generates a .h file for the specified property.
\o/!

[aazzam, 2016-12-15 23:24:13]

The CQ bit was checked by aazzam@google.com

[aazzam, 2016-12-15 23:24:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
alancutter@chromium.org
Link to the patchset: https://codereview.chromium.org/2578773002/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2016-12-15 23:25:01]

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, 2016-12-15 23:49:53]

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

[commit-bot: I haz the power, 2016-12-15 23:49:54]

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...)

[aazzam, 2016-12-16 01:09:03]

The CQ bit was checked by aazzam@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-16 01:10:05]

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, 2016-12-16 01:51:59]

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

[commit-bot: I haz the power, 2016-12-16 01:52:00]

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...)

[aazzam, 2016-12-16 02:52:23]

The CQ bit was checked by aazzam@google.com to run a CQ dry run

[aazzam, 2016-12-16 02:53:12]

The CQ bit was unchecked by aazzam@google.com

[aazzam, 2016-12-16 03:31:22]

The CQ bit was checked by aazzam@google.com

[aazzam, 2016-12-16 03:31:23]

The patchset sent to the CQ was uploaded after l-g-t-m from sashab@chromium.org,
alancutter@chromium.org
Link to the patchset: https://codereview.chromium.org/2578773002/#ps100001
(title: "Removed generated .h files from BUILD")

[commit-bot: I haz the power, 2016-12-16 03:32:53]

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, 2016-12-16 07:00:46]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1481859082904690,
"parent_rev": "55193785ad5c2b79504e1194132f8319900e1cb3", "commit_rev":
"1e46b78ad70b159a6ff4356d796daa1d1112f184"}

[commit-bot: I haz the power, 2016-12-16 07:01:20]

Description was changed from

==========
Made a generator for .h files for implementations of CSSPropertyAPI.h

Renamed make_css_property_descriptor.py to make_css_property_api.py and
modified it so that it also generates API .h files. Added the template
file CSSPropertyAPI.h.tmpl.

To generate the .h file for a property, add the api_class flag to the
property in CSSProperties.in.

BUG=668012
==========

to

==========
Made a generator for .h files for implementations of CSSPropertyAPI.h

Renamed make_css_property_descriptor.py to make_css_property_api.py and
modified it so that it also generates API .h files. Added the template
file CSSPropertyAPI.h.tmpl.

To generate the .h file for a property, add the api_class flag to the
property in CSSProperties.in.

BUG=668012

Review-Url: https://codereview.chromium.org/2578773002
==========

[commit-bot: I haz the power, 2016-12-16 07:01:21]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-12-16 07:04:41]

Description was changed from

==========
Made a generator for .h files for implementations of CSSPropertyAPI.h

Renamed make_css_property_descriptor.py to make_css_property_api.py and
modified it so that it also generates API .h files. Added the template
file CSSPropertyAPI.h.tmpl.

To generate the .h file for a property, add the api_class flag to the
property in CSSProperties.in.

BUG=668012

Review-Url: https://codereview.chromium.org/2578773002
==========

to

==========
Made a generator for .h files for implementations of CSSPropertyAPI.h

Renamed make_css_property_descriptor.py to make_css_property_api.py and
modified it so that it also generates API .h files. Added the template
file CSSPropertyAPI.h.tmpl.

To generate the .h file for a property, add the api_class flag to the
property in CSSProperties.in.

BUG=668012

Committed: https://crrev.com/de5be95f7da7d464a16f1a1fa930bd4340063f94
Cr-Commit-Position: refs/heads/master@{#439053}
==========

[commit-bot: I haz the power, 2016-12-16 07:04:42]

Patchset 6 (id:??) landed as
https://crrev.com/de5be95f7da7d464a16f1a1fa930bd4340063f94
Cr-Commit-Position: refs/heads/master@{#439053}