I noticed that there is a WebKitTools/gdb/webcore.py. It would be better to support more types for GDB lovers.
Created attachment 59241 [details] support-wtf-vector
Attachment 59241 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebKitTools/gdb/wtf.py:41: expected 2 blank lines, found 1 [pep8/E302] [5] WebKitTools/gdb/wtf.py:45: whitespace before '(' [pep8/E211] [5] WebKitTools/gdb/wtf.py:92: indentation is not a multiple of four [pep8/E111] [5] Total errors found: 3 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 59338 [details] support-wtf-vector-fix-style
I've fixed the style. (In reply to comment #3) > Created an attachment (id=59338) [details] > support-wtf-vector-fix-style (In reply to comment #2) > Attachment 59241 [details] did not pass style-queue: > > Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 > WebKitTools/gdb/wtf.py:41: expected 2 blank lines, found 1 [pep8/E302] [5] > WebKitTools/gdb/wtf.py:45: whitespace before '(' [pep8/E211] [5] > WebKitTools/gdb/wtf.py:92: indentation is not a multiple of four [pep8/E111] [5] > Total errors found: 3 in 2 files > > > If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 59338 [details] support-wtf-vector-fix-style r- for minor issues. WebKitTools/gdb/wtf.py:98 + build_pritty_printers_dict() pretty WebKitTools/gdb/wtf.py:96 + pretty_printers_dict = {} It would be better not to use a global variable? Maybe pretty_printers = build_pretty_printers() gdb.pretty_printers.append(lambda val: lookup_function(pretty_printers, val)) (I'm not 100% sure if it's OK to pass lambda to gdb.pretty_printers though) WebKitTools/gdb/wtf.py:43 + "Print a WTF::Vector" Could you add a document about how each members will be used? I guess both children and to_string will be called for GDB's p command when display_hint is 'array'? Maybe putting URL for GDB's python document would be nice. WebKitTools/gdb/wtf.py:59 + elt = self.item.dereference() I think element would be better. Usually WebKit code doesn't like abbreviations.
Though I'm not good at GDB's python extension, I guess I will r+ the change in the next iteration as this change harms nothing and looks good to me. I'm CCing Evan as he could have some cool ideas to improve this patch.
I also don't know a lot about GDB extensions, but the change looks good to me! You might want to mention it on webkit-dev once it's landed.
Created attachment 62563 [details] support-wtf-vector-typo-fixed-and-add-comments
Thank you for the review. (In reply to comment #5) > (From update of attachment 59338 [details]) > r- for minor issues. > > WebKitTools/gdb/wtf.py:98 > + build_pritty_printers_dict() > pretty Done. Thank you. > > WebKitTools/gdb/wtf.py:96 > + pretty_printers_dict = {} > It would be better not to use a global variable? Maybe > > pretty_printers = build_pretty_printers() > gdb.pretty_printers.append(lambda val: lookup_function(pretty_printers, val)) > > (I'm not 100% sure if it's OK to pass lambda to gdb.pretty_printers though) I don't think it is worth hiding 'pretty_printer_dict' from global scope. 'pretty_printer_dict' is global, but that is global only within this module. I'd like to leave that as is because that won't conflict any variables defined in other modules. > > > WebKitTools/gdb/wtf.py:43 > + "Print a WTF::Vector" > Could you add a document about how each members will be used? I guess both children and to_string will be called for GDB's p command when display_hint is 'array'? Maybe putting URL for GDB's python document would be nice. Sure. I've added comments mentioning the url and usage. > > WebKitTools/gdb/wtf.py:59 > + elt = self.item.dereference() > I think element would be better. Usually WebKit code doesn't like abbreviations. Done. Thank you.
Comment on attachment 62563 [details] support-wtf-vector-typo-fixed-and-add-comments Looks good. Thanks for your update! > I don't think it is worth hiding 'pretty_printer_dict' from global scope. 'pretty_printer_dict' is global, but that is global only within this module. I'd like to leave that as is because that won't conflict any variables defined in other modules. I still like not to use global variables because I was a bit surprised this code seemed to use a variable without declaration. At first, I guessed pretty_printers_dict is something special in GDB extensions. But it's OK as is.
(In reply to comment #7) > I also don't know a lot about GDB extensions, but the change looks good to me! You might want to mention it on webkit-dev once it's landed. Thanks for your comment!
Comment on attachment 62563 [details] support-wtf-vector-typo-fixed-and-add-comments Clearing flags on attachment: 62563 Committed r64044: <http://trac.webkit.org/changeset/64044>
All reviewed patches have been landed. Closing bug.