[Webkit-unassigned] [Bug 40909] Support WTF::Vector pretty printer for GDB 7

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=40909





--- Comment #9 from Hayato Ito <hayato at chromium.org>  2010-07-26 06:20:27 PST ---
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.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.