[chromedriver] Fix mobile device embedding script

Fix mobile device embedding script:

* Keep lines within 80 chars, taking care around backslashes.
* Surround key names with '"'.
* Put a ',' between phones and tablets when concatenating.

Committed: https://crrev.com/18e2ed3382ed32dfbee0727f34ea34ac0d3a0041
Cr-Commit-Position: refs/heads/master@{#312914}

[srawlins, 2014-12-08 19:18:40]

srawlins@google.com changed reviewers:
+ samuong@google.com

[samuong, 2014-12-11 00:20:41]

Could you add "[chromedriver]" to the front of the issue subject and
description?

I'm happy with the quoting of key names and adding the ",". Let me know what you
think about splitting lines using clang-format instead.

[samuong, 2014-12-11 00:22:18]

samuong@chromium.org changed reviewers:
+ samuong@chromium.org

[samuong, 2014-12-11 00:22:19]

https://codereview.chromium.org/749353003/diff/1/chrome/test/chromedriver/cpp...
File chrome/test/chromedriver/cpp_source.py (right):

https://codereview.chromium.org/749353003/diff/1/chrome/test/chromedriver/cpp...
chrome/test/chromedriver/cpp_source.py:32: '// found in the LICENSE file.']) %
datetime.date.today().year
Now that we're kicking off the generation of (some of) these files manually, I
think we should have a comment explaining that these are generated files and how
to regenerate them. Maybe something like:

// This file was generated at (datetime.datetime()) by running ('
'.join(sys.argv))

That would make it easier for future chromedriver devs to regenerate the mobile
device list if they find it is out of date.

https://codereview.chromium.org/749353003/diff/1/chrome/test/chromedriver/cpp...
chrome/test/chromedriver/cpp_source.py:73: lines.append('    "%s"' % contents)
Maybe an easier way to do this is to just pump everything through clang-format?
This doesn't even really need to be automated. People could just do:

$ chrome/test/chromedriver/embed_mobile_devices_in_cpp.py
--directory=chrome/test/chromedriver/chrome/
third_party/WebKit/Source/devtools/front_end/toolbox/OverridesUI.js
$ clang-format -i chrome/test/chromedriver/chrome/mobile_device_list.cc

The nice thing about clang-format is that it breaks the lines between words,
which makes the generated code a bit easier to read, and also ensures that there
are no syntax errors in the generated file.

[srawlins, 2014-12-12 23:41:29]

Done and done.

https://codereview.chromium.org/749353003/diff/1/chrome/test/chromedriver/cpp...
File chrome/test/chromedriver/cpp_source.py (right):

https://codereview.chromium.org/749353003/diff/1/chrome/test/chromedriver/cpp...
chrome/test/chromedriver/cpp_source.py:32: '// found in the LICENSE file.']) %
datetime.date.today().year
On 2014/12/11 00:22:19, samuong wrote:
> Now that we're kicking off the generation of (some of) these files manually, I
> think we should have a comment explaining that these are generated files and
how
> to regenerate them. Maybe something like:
> 
> // This file was generated at (datetime.datetime()) by running ('
> '.join(sys.argv))
> 
> That would make it easier for future chromedriver devs to regenerate the
mobile
> device list if they find it is out of date.

Done.

https://codereview.chromium.org/749353003/diff/1/chrome/test/chromedriver/cpp...
chrome/test/chromedriver/cpp_source.py:73: lines.append('    "%s"' % contents)
On 2014/12/11 00:22:19, samuong wrote:
> Maybe an easier way to do this is to just pump everything through
clang-format?
> This doesn't even really need to be automated. People could just do:
> 
> $ chrome/test/chromedriver/embed_mobile_devices_in_cpp.py
> --directory=chrome/test/chromedriver/chrome/
> third_party/WebKit/Source/devtools/front_end/toolbox/OverridesUI.js
> $ clang-format -i chrome/test/chromedriver/chrome/mobile_device_list.cc
> 
> The nice thing about clang-format is that it breaks the lines between words,
> which makes the generated code a bit easier to read, and also ensures that
there
> are no syntax errors in the generated file.

Done.

[samuong, 2015-01-20 22:02:33]

https://codereview.chromium.org/749353003/diff/20001/chrome/test/chromedriver...
File chrome/test/chromedriver/cpp_source.py (right):

https://codereview.chromium.org/749353003/diff/20001/chrome/test/chromedriver...
chrome/test/chromedriver/cpp_source.py:30: '// found in the LICENSE file.',
nit: can we add one blank line between the copyright message and the generated
file warning?

https://codereview.chromium.org/749353003/diff/20001/chrome/test/chromedriver...
File chrome/test/chromedriver/embed_mobile_devices_in_cpp.py (right):

https://codereview.chromium.org/749353003/diff/20001/chrome/test/chromedriver...
chrome/test/chromedriver/embed_mobile_devices_in_cpp.py:69: '    clang-format -i
%s/mobile_device_list.h' % output_dir]))
It'd be nice if we could automate this. Can you use subprocess.Popen here
instead? "clang-format -i" reformats the file in-place, so you don't need to
mess around with pipes or temporary directories.

[srawlins, 2015-01-22 20:15:58]

Here you go, Sam. I tested it, looks great!

https://codereview.chromium.org/749353003/diff/20001/chrome/test/chromedriver...
File chrome/test/chromedriver/cpp_source.py (right):

https://codereview.chromium.org/749353003/diff/20001/chrome/test/chromedriver...
chrome/test/chromedriver/cpp_source.py:30: '// found in the LICENSE file.',
On 2015/01/20 22:02:33, samuong wrote:
> nit: can we add one blank line between the copyright message and the generated
> file warning?

Done.

https://codereview.chromium.org/749353003/diff/20001/chrome/test/chromedriver...
File chrome/test/chromedriver/embed_mobile_devices_in_cpp.py (right):

https://codereview.chromium.org/749353003/diff/20001/chrome/test/chromedriver...
chrome/test/chromedriver/embed_mobile_devices_in_cpp.py:69: '    clang-format -i
%s/mobile_device_list.h' % output_dir]))
On 2015/01/20 22:02:33, samuong wrote:
> It'd be nice if we could automate this. Can you use subprocess.Popen here
> instead? "clang-format -i" reformats the file in-place, so you don't need to
> mess around with pipes or temporary directories.

Done.

[samuong, 2015-01-22 20:21:38]

On 2015/01/22 20:15:58, srawlins wrote:
> Here you go, Sam. I tested it, looks great!
> 
>
https://codereview.chromium.org/749353003/diff/20001/chrome/test/chromedriver...
> File chrome/test/chromedriver/cpp_source.py (right):
> 
>
https://codereview.chromium.org/749353003/diff/20001/chrome/test/chromedriver...
> chrome/test/chromedriver/cpp_source.py:30: '// found in the LICENSE file.',
> On 2015/01/20 22:02:33, samuong wrote:
> > nit: can we add one blank line between the copyright message and the
generated
> > file warning?
> 
> Done.
> 
>
https://codereview.chromium.org/749353003/diff/20001/chrome/test/chromedriver...
> File chrome/test/chromedriver/embed_mobile_devices_in_cpp.py (right):
> 
>
https://codereview.chromium.org/749353003/diff/20001/chrome/test/chromedriver...
> chrome/test/chromedriver/embed_mobile_devices_in_cpp.py:69: '    clang-format
-i
> %s/mobile_device_list.h' % output_dir]))
> On 2015/01/20 22:02:33, samuong wrote:
> > It'd be nice if we could automate this. Can you use subprocess.Popen here
> > instead? "clang-format -i" reformats the file in-place, so you don't need to
> > mess around with pipes or temporary directories.
> 
> Done.

Thanks Sam. LGTM with just one nit.

[samuong, 2015-01-22 20:21:48]

https://codereview.chromium.org/749353003/diff/40001/chrome/test/chromedriver...
File chrome/test/chromedriver/embed_mobile_devices_in_cpp.py (right):

https://codereview.chromium.org/749353003/diff/40001/chrome/test/chromedriver...
chrome/test/chromedriver/embed_mobile_devices_in_cpp.py:8: The script requires
the OverridesUI file from WebKit that lists the known
nit: rather than say "the OverridesUI file from WebKit", maybe just give the
path?

[srawlins, 2015-01-23 19:21:56]

The CQ bit was checked by srawlins@google.com

[commit-bot: I haz the power, 2015-01-23 19:23:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/749353003/60001

[commit-bot: I haz the power, 2015-01-23 20:12:50]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-01-23 20:14:42]

Patchset 4 (id:??) landed as
https://crrev.com/18e2ed3382ed32dfbee0727f34ea34ac0d3a0041
Cr-Commit-Position: refs/heads/master@{#312914}