url_formatter: Update comments for clarity.

url_formatter: Update comments for clarity.

- Separate comment describing each character in the exclusion list.
- Documentation for struct fields in test.

Review-Url: https://codereview.chromium.org/2595723003
Cr-Commit-Position: refs/heads/master@{#471673}
Committed: https://chromium.googlesource.com/chromium/src/+/7455b06b8b993933393900c6624534335d412e4b

[Matt Giuca, 2017-02-03 05:18:24]

mgiuca@chromium.org changed reviewers:
+ pkasting@chromium.org

[Matt Giuca, 2017-02-03 05:18:25]

Hi Peter,

I wrote this CL as a side thing while working on r440282 but I totally forgot to
mail it out :(

I wanted to rearrange these comments so they scale better if we want to add or
remove items later (instead of just being a big paragraph blob).

The Python function might be a bit much to have inside a comment inside C++
code, but it was very helpful in generating the tests I added in r440282 and
maybe will help someone in the future. (But didn't warrant writing and landing a
whole Python tool.) WDYT?

[Peter Kasting, 2017-02-06 21:54:14]

LGTM

https://codereview.chromium.org/2595723003/diff/20001/components/url_formatte...
File components/url_formatter/url_formatter_unittest.cc (right):

https://codereview.chromium.org/2595723003/diff/20001/components/url_formatte...
components/url_formatter/url_formatter_unittest.cc:50: //         (idna_input,
unicode_output, repr(unicode_allowed).lower()))
You said this didn't warrant a separate tool, but it honestly seems like the
better route would be to just go ahead and check this script in in a tools
directory, then have the comment note that it can be used?  I dunno.

[jungshik at Google, 2017-03-10 20:33:36]

On 2017/02/06 21:54:14, Peter Kasting wrote:
> LGTM
> 
>
https://codereview.chromium.org/2595723003/diff/20001/components/url_formatte...
> File components/url_formatter/url_formatter_unittest.cc (right):
> 
>
https://codereview.chromium.org/2595723003/diff/20001/components/url_formatte...
> components/url_formatter/url_formatter_unittest.cc:50: //         (idna_input,
> unicode_output, repr(unicode_allowed).lower()))
> You said this didn't warrant a separate tool, but it honestly seems like the
> better route would be to just go ahead and check this script in in a tools
> directory, then have the comment note that it can be used?  I dunno.

Thank you, Matt. I also had a similar script locally. Perhaps, it's a good idea
to check that in as Peter said?

[Matt Giuca, 2017-03-14 02:17:01]

On 2017/03/10 20:33:36, jungshik at Google wrote:
> On 2017/02/06 21:54:14, Peter Kasting wrote:
> > LGTM
> > 
> >
>
https://codereview.chromium.org/2595723003/diff/20001/components/url_formatte...
> > File components/url_formatter/url_formatter_unittest.cc (right):
> > 
> >
>
https://codereview.chromium.org/2595723003/diff/20001/components/url_formatte...
> > components/url_formatter/url_formatter_unittest.cc:50: //        
(idna_input,
> > unicode_output, repr(unicode_allowed).lower()))
> > You said this didn't warrant a separate tool, but it honestly seems like the
> > better route would be to just go ahead and check this script in in a tools
> > directory, then have the comment note that it can be used?  I dunno.
> 
> Thank you, Matt. I also had a similar script locally. Perhaps, it's a good
idea
> to check that in as Peter said?

Yeah, I have been planning to do that which is why I haven't just landed this
CL. I just need to get some other things off my plate before doing this, though.
(Since checking in a script means it needs to be a bit more robust than what
I've got here in this comment.)

[Matt Giuca, 2017-05-15 04:20:29]

Description was changed from

==========
url_formatter: Update comments for clarity.

- Separate comment describing each character in the exclusion list.
- Documentation for struct fields in test.
- Small Python function (in a comment) that is helpful in generating
  test cases.
==========

to

==========
url_formatter: Update comments for clarity.

- Separate comment describing each character in the exclusion list.
- Documentation for struct fields in test.
==========

[Matt Giuca, 2017-05-15 04:24:59]

OK I finally got around to this. I've converted the Python function into a full
script program (and made it much more robust).

Because it's big, I split it out into a separate CL:
https://codereview.chromium.org/2880983002

[Matt Giuca, 2017-05-15 04:25:06]

The CQ bit was checked by mgiuca@chromium.org

[Matt Giuca, 2017-05-15 04:25:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/2595723003/#ps80001
(title: "Remove IDN test case generator (for future CL).")

[commit-bot: I haz the power, 2017-05-15 04:25:14]

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-05-15 05:28:58]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1494822306054600,
"parent_rev": "b7a5c55bb3f841ff77446c65edfe56bcc1f893a4", "commit_rev":
"7455b06b8b993933393900c6624534335d412e4b"}

[commit-bot: I haz the power, 2017-05-15 05:29:11]

Description was changed from

==========
url_formatter: Update comments for clarity.

- Separate comment describing each character in the exclusion list.
- Documentation for struct fields in test.
==========

to

==========
url_formatter: Update comments for clarity.

- Separate comment describing each character in the exclusion list.
- Documentation for struct fields in test.

Review-Url: https://codereview.chromium.org/2595723003
Cr-Commit-Position: refs/heads/master@{#471673}
Committed:
https://chromium.googlesource.com/chromium/src/+/7455b06b8b993933393900c66245...
==========

[commit-bot: I haz the power, 2017-05-15 05:29:12]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/7455b06b8b993933393900c66245...