Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1371573002:
Get diff that made by make_diff.py and print it by color on a shell. (Closed)
Descriptionbindings: Add print_idl_diff.py
Print a diff that is generated by generate_idl_diff.py with color on a terminal.
BUG=530402
Committed: https://crrev.com/0be80d6e5232c2a7e3b12ea8078ab2471b216cb6
Cr-Commit-Position: refs/heads/master@{#350791}
Patch Set 1 #
Total comments: 28
Patch Set 2 : Modified some nits #
Total comments: 9
Patch Set 3 : Updated the comment of print_items() #
Total comments: 79
Patch Set 4 : Updated based on haraken-san's advice #Messages
Total messages: 18 (3 generated)
shimadaa@google.com changed reviewers: + bashi@chromium.org, haraken@chromium.org, yukishiino@chromium.org
LGTM with nits. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... File third_party/WebKit/Source/bindings/scripts/print_idl_diff.py (right): https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:15: Specify how to sort. Either by using diffing tags or in alphabetical order. nit: wrap line after 'alphabetical'. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:103: TODO(shimadaa): This class doesn't work on Windows. Provides a way to suppress escape sequences. nit: wrap line after 'a way to' https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:116: """Reset text's color to default and output it to sys.stdout. nit: remove 'and output it to sys.stdout'. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:121: """Change text's color by specifing arguments and output it to sys.stdout. nit: remove 'and output it to sys.stdout'. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:123: color: A value that represents text's color. Mapping colors into values is below. how about: color: A color to change. It should be one of |COLORS|. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:156: """Group members of a given argument (an interface or a list of members) by tags. nit: wrap line after 'by' https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:158: interface_or_member_list: Interface name's list or "members" object's list. nit: wrap line after "object's" https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:162: unspecified: A list that consists of neither removed ones nor added ones. nit: wrap line after 'added' https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:194: return (sorted_interface_name) nit: remove paresis. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:207: else: nit: You can remmove else clause. if DIFF_TAG in interface: return interface for member_type in EXTATTRIBUTES_AND_MEMBER_TYPES: ... https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:239: sorted_members = sorted(interface[member_type], key=lambda member: member['Name']) nit: wrap line after ',' https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:261: """Change color based on diff_tag and output prefix ('+' or '-') if |member| is added/deleted. nit: wrap line after |member| https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:303: def print_items(items, callback, out): nit: Please add a comment to describe what this function does. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:400: def usage(): nit: Add a comment. """Show usage."""
I modified some nits. I have no confident in print_items()'s comment. Thank you in advance. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... File third_party/WebKit/Source/bindings/scripts/print_idl_diff.py (right): https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:15: Specify how to sort. Either by using diffing tags or in alphabetical order. On 2015/09/25 06:40:58, bashi1 wrote: > nit: wrap line after 'alphabetical'. Done. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:103: TODO(shimadaa): This class doesn't work on Windows. Provides a way to suppress escape sequences. On 2015/09/25 06:40:58, bashi1 wrote: > nit: wrap line after 'a way to' Done. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:116: """Reset text's color to default and output it to sys.stdout. On 2015/09/25 06:40:58, bashi1 wrote: > nit: remove 'and output it to sys.stdout'. Done. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:121: """Change text's color by specifing arguments and output it to sys.stdout. On 2015/09/25 06:40:58, bashi1 wrote: > nit: remove 'and output it to sys.stdout'. Done. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:123: color: A value that represents text's color. Mapping colors into values is below. On 2015/09/25 06:40:58, bashi1 wrote: > how about: > > color: A color to change. It should be one of |COLORS|. Done. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:156: """Group members of a given argument (an interface or a list of members) by tags. On 2015/09/25 06:40:58, bashi1 wrote: > nit: wrap line after 'by' Done. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:158: interface_or_member_list: Interface name's list or "members" object's list. On 2015/09/25 06:40:58, bashi1 wrote: > nit: wrap line after "object's" Done. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:162: unspecified: A list that consists of neither removed ones nor added ones. On 2015/09/25 06:40:58, bashi1 wrote: > nit: wrap line after 'added' Done. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:194: return (sorted_interface_name) On 2015/09/25 06:40:58, bashi1 wrote: > nit: remove paresis. Done. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:207: else: On 2015/09/25 06:40:58, bashi1 wrote: > nit: You can remmove else clause. > > if DIFF_TAG in interface: > return interface > > for member_type in EXTATTRIBUTES_AND_MEMBER_TYPES: > ... Done. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:239: sorted_members = sorted(interface[member_type], key=lambda member: member['Name']) On 2015/09/25 06:40:58, bashi1 wrote: > nit: wrap line after ',' Done. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:261: """Change color based on diff_tag and output prefix ('+' or '-') if |member| is added/deleted. On 2015/09/25 06:40:58, bashi1 wrote: > nit: wrap line after |member| Done. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:303: def print_items(items, callback, out): On 2015/09/25 06:40:58, bashi1 wrote: > nit: Please add a comment to describe what this function does. Done. https://codereview.chromium.org/1371573002/diff/1/third_party/WebKit/Source/b... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:400: def usage(): On 2015/09/25 06:40:58, bashi1 wrote: > nit: Add a comment. > > """Show usage.""" Done.
LGTM https://codereview.chromium.org/1371573002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/bindings/scripts/print_idl_diff.py (right): https://codereview.chromium.org/1371573002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:12: expressing a diff between two defferent Chrome versions.The structure nit: a space just before "The structure" https://codereview.chromium.org/1371573002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:96: ] 'Name': '...' should be here, too? https://codereview.chromium.org/1371573002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:165: removed: A list that consists of removed ones. I'd recommend to explicitly say that the return value is a tuple. Returns: A tuple of (removed, added, unchanged) where removed: A list that ... added: A list that ... unchanged: A list that ... https://codereview.chromium.org/1371573002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:413: sys.stdout.write('Usage: print_diff.py <diff_file.json> <TAG>/<ALPHABET>\n') <"TAG" | "ALPHABET">
https://codereview.chromium.org/1371573002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/bindings/scripts/print_idl_diff.py (right): https://codereview.chromium.org/1371573002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:12: expressing a diff between two defferent Chrome versions.The structure On 2015/09/25 07:12:37, Yuki wrote: > nit: a space just before "The structure" Done. https://codereview.chromium.org/1371573002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:96: ] On 2015/09/25 07:12:37, Yuki wrote: > 'Name': '...' should be here, too? Done. https://codereview.chromium.org/1371573002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:165: removed: A list that consists of removed ones. On 2015/09/25 07:12:37, Yuki wrote: > I'd recommend to explicitly say that the return value is a tuple. > > Returns: > A tuple of (removed, added, unchanged) where > removed: A list that ... > added: A list that ... > unchanged: A list that ... Done. https://codereview.chromium.org/1371573002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:413: sys.stdout.write('Usage: print_diff.py <diff_file.json> <TAG>/<ALPHABET>\n') On 2015/09/25 07:12:37, Yuki wrote: > <"TAG" | "ALPHABET"> Done.
Could you upload the latest one?
https://codereview.chromium.org/1371573002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/bindings/scripts/print_idl_diff.py (right): https://codereview.chromium.org/1371573002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:311: """Print ', ' in ExtAttributes or Arguments. How about: """Calls |callback| for each item in |items|, printing commas between |callback| calls. """
lgtm
The CQ bit was checked by bashi@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bashi@chromium.org Link to the patchset: https://codereview.chromium.org/1371573002/#ps40001 (title: "Updated the comment of print_items()")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1371573002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1371573002/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/0be80d6e5232c2a7e3b12ea8078ab2471b216cb6 Cr-Commit-Position: refs/heads/master@{#350791}
Message was sent while issue was closed.
LGTM Most comments are just about English nits. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/bindings/scripts/print_idl_diff.py (right): https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:7: Before printing, sort the idl data of the diff in alphabetical order or by sort the diff in the alphabetical order or the order of diffing tags https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:11: Output of generate_idl_diff.py. A json file consists of a dictionary The json file contains a dictionary that represents... https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:12: expressing a diff between two defferent Chrome versions. The structure different Chromium versions https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:15: Specify how to sort. Either by using diffing tags or in alphabetical "ALPHABET" or "TAG" https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:31: The deffference between input structure of generate_idl_diff.py and The difference between the input structure... https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:33: The diffing tags are included in a parts that have a diff. I'd remove this sentence. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:105: """This class decorates text with colors and outputs it to sys.stdout. This class outputs a colored text to sys.stdout. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:106: TODO(shimadaa): This class doesn't work on Windows. Provides a way to TODO(bashi) https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:106: TODO(shimadaa): This class doesn't work on Windows. Provides a way to Provide https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:127: color: A color to change. It should be one of |COLORS|. A new color. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:135: """Print text to use for line breaks. Print text with a line-break. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:140: """Print text not to use for line breaks. Print text without a line-break. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:160: """Group members of a given argument (an interface or a list of members) by Group members of |interface_or_member_list| by tags https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:163: interface_or_member_list: Interface name's list or "members" object's A list of interface names or a list of "members" https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:167: removed: A list that consists of removed ones. A list of removed members https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:168: added: A list that consists of added ones. A list of added members https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:169: unspecified: A list that consists of neither removed ones nor added A list of other members https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:187: """Sort the order of interface names like below. Sort interface names as follows. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:188: [a interface name of "interface" deleted whole names of deleted "interface"s https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:189: -> a interface name of "interface" added whole names of added "interface"s https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:190: -> a interface name of "interface" changed part] names of other "interface"s https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:192: interfaces: An "interface" objects. "interface" objects https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:194: A list consists of interface names sorted A list of sorted interface names https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:201: sorted_interface_name = removed + added + unspecified sorted_interface_names https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:206: """Sort a "members" object by using diff_tag. Sort members of a given interface in the order of diffing tags https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:223: """Sort an "interfaces" object expressing a diff by using diff_tag. Sort an "interfaces" object in the order of diffing tags https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:238: """Sort a "members" object in alphabetical order. in the alphabetical order https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:253: """Sort diff in alphabetical order. Sort an "interfaces" object in the alphabetical order https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:257: A sorted "interfaces" object in alphabetical order A sorted "interfaces" object https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:268: def change_color_based_on_tag(member, out): change_color_based_on_tag => print_member_with_color ? https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:269: """Change color based on diff_tag and output prefix ('+' or '-') if |member| Print the "member" with a colored text. '+' is added to an added "member". '-' is added to a removed "member". https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:289: A list included as a value of "member" object named "ExtAttributes" A list of "ExtAttributes" in the "interface" object https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:300: A list included as a value of "member" object named "Consts" A list of "Consts" of the "interface" object https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:326: def print_extattributes_of_member(extattributes, out): print_extattributes_in_member https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:326: def print_extattributes_of_member(extattributes, out): Why do we need to distinguish print_extattributes_of_member from print_extattributes? https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:329: A list included as a value of "ExtAttributes" of "member" object A list of "ExtAttributes" in the "member" object https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:342: A list included as a value of "member" object named "Attributes" A list of "Attributes" in the "interface" object https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:356: Args: A list included as a value of "Arguments" of "member" object named A list of "Arguments". https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:370: A list included as a value of "member" object named "Operations" A list of "Operations" https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:414: def usage(): print_usage
Message was sent while issue was closed.
Updated based on haraken-san's advice. I think that this is the last updating. Thank you so much. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/bindings/scripts/print_idl_diff.py (right): https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:7: Before printing, sort the idl data of the diff in alphabetical order or by On 2015/09/25 10:30:19, haraken wrote: > > sort the diff in the alphabetical order or the order of diffing tags Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:11: Output of generate_idl_diff.py. A json file consists of a dictionary On 2015/09/25 10:30:19, haraken wrote: > > The json file contains a dictionary that represents... Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:12: expressing a diff between two defferent Chrome versions. The structure On 2015/09/25 10:30:18, haraken wrote: > > different Chromium versions Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:15: Specify how to sort. Either by using diffing tags or in alphabetical On 2015/09/25 10:30:18, haraken wrote: > > "ALPHABET" or "TAG" Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:31: The deffference between input structure of generate_idl_diff.py and On 2015/09/25 10:30:19, haraken wrote: > > The difference between the input structure... Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:33: The diffing tags are included in a parts that have a diff. On 2015/09/25 10:30:18, haraken wrote: > > I'd remove this sentence. Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:105: """This class decorates text with colors and outputs it to sys.stdout. On 2015/09/25 10:30:19, haraken wrote: > > This class outputs a colored text to sys.stdout. Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:106: TODO(shimadaa): This class doesn't work on Windows. Provides a way to On 2015/09/25 10:30:19, haraken wrote: > > TODO(bashi) Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:106: TODO(shimadaa): This class doesn't work on Windows. Provides a way to On 2015/09/25 10:30:19, haraken wrote: > > Provide Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:127: color: A color to change. It should be one of |COLORS|. On 2015/09/25 10:30:19, haraken wrote: > > A new color. Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:135: """Print text to use for line breaks. On 2015/09/25 10:30:19, haraken wrote: > > Print text with a line-break. Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:140: """Print text not to use for line breaks. On 2015/09/25 10:30:18, haraken wrote: > > Print text without a line-break. Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:160: """Group members of a given argument (an interface or a list of members) by On 2015/09/25 10:30:20, haraken wrote: > > Group members of |interface_or_member_list| by tags Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:163: interface_or_member_list: Interface name's list or "members" object's On 2015/09/25 10:30:20, haraken wrote: > > A list of interface names or a list of "members" Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:167: removed: A list that consists of removed ones. On 2015/09/25 10:30:18, haraken wrote: > > A list of removed members Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:168: added: A list that consists of added ones. On 2015/09/25 10:30:19, haraken wrote: > > A list of added members Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:169: unspecified: A list that consists of neither removed ones nor added On 2015/09/25 10:30:19, haraken wrote: > > A list of other members Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:187: """Sort the order of interface names like below. On 2015/09/25 10:30:19, haraken wrote: > > Sort interface names as follows. Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:188: [a interface name of "interface" deleted whole On 2015/09/25 10:30:19, haraken wrote: > > names of deleted "interface"s Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:189: -> a interface name of "interface" added whole On 2015/09/25 10:30:20, haraken wrote: > > names of added "interface"s Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:190: -> a interface name of "interface" changed part] On 2015/09/25 10:30:19, haraken wrote: > > names of other "interface"s Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:192: interfaces: An "interface" objects. On 2015/09/25 10:30:19, haraken wrote: > > "interface" objects Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:194: A list consists of interface names sorted On 2015/09/25 10:30:19, haraken wrote: > > A list of sorted interface names Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:201: sorted_interface_name = removed + added + unspecified On 2015/09/25 10:30:20, haraken wrote: > > sorted_interface_names Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:206: """Sort a "members" object by using diff_tag. On 2015/09/25 10:30:19, haraken wrote: > > Sort members of a given interface in the order of diffing tags Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:223: """Sort an "interfaces" object expressing a diff by using diff_tag. On 2015/09/25 10:30:19, haraken wrote: > > Sort an "interfaces" object in the order of diffing tags Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:238: """Sort a "members" object in alphabetical order. On 2015/09/25 10:30:19, haraken wrote: > > in the alphabetical order Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:253: """Sort diff in alphabetical order. On 2015/09/25 10:30:18, haraken wrote: > > Sort an "interfaces" object in the alphabetical order Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:257: A sorted "interfaces" object in alphabetical order On 2015/09/25 10:30:20, haraken wrote: > > A sorted "interfaces" object Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:268: def change_color_based_on_tag(member, out): On 2015/09/25 10:30:18, haraken wrote: > > change_color_based_on_tag => print_member_with_color ? Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:269: """Change color based on diff_tag and output prefix ('+' or '-') if |member| On 2015/09/25 10:30:19, haraken wrote: > > Print the "member" with a colored text. '+' is added to an added "member". '-' > is added to a removed "member". > Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:289: A list included as a value of "member" object named "ExtAttributes" On 2015/09/25 10:30:19, haraken wrote: > > A list of "ExtAttributes" in the "interface" object > Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:300: A list included as a value of "member" object named "Consts" On 2015/09/25 10:30:19, haraken wrote: > > A list of "Consts" of the "interface" object > Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:326: def print_extattributes_of_member(extattributes, out): On 2015/09/25 10:30:19, haraken wrote: > > print_extattributes_in_member Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:326: def print_extattributes_of_member(extattributes, out): On 2015/09/25 10:30:19, haraken wrote: > > Why do we need to distinguish print_extattributes_of_member from > print_extattributes? Sorry I don't have time to modify... https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:329: A list included as a value of "ExtAttributes" of "member" object On 2015/09/25 10:30:19, haraken wrote: > > A list of "ExtAttributes" in the "member" object Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:342: A list included as a value of "member" object named "Attributes" On 2015/09/25 10:30:19, haraken wrote: > > A list of "Attributes" in the "interface" object > Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:356: Args: A list included as a value of "Arguments" of "member" object named On 2015/09/25 10:30:19, haraken wrote: > > A list of "Arguments". Done. https://codereview.chromium.org/1371573002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/bindings/scripts/print_idl_diff.py:370: A list included as a value of "member" object named "Operations" On 2015/09/25 10:30:20, haraken wrote: > > A list of "Operations" Done.
Message was sent while issue was closed.
On 2015/09/25 11:31:28, shimadaa wrote: > Updated based on haraken-san's advice. I think that this is the last updating. > Thank you so much. > Thanks for the final update! LGTM I'll land this for you :) |
