WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
38664
Web Inspector: add a "table" method to console, to allow output of tabular data
https://bugs.webkit.org/show_bug.cgi?id=38664
Summary
Web Inspector: add a "table" method to console, to allow output of tabular data
Patrick Mueller
Reported
2010-05-06 10:59:22 PDT
Requirement comes from the wildfire/firephp work. See
bug 30897
more info. Both Wildfire -
http://framework.zend.com/manual/en/zend.log.writers.html#zend.log.writers.firebug
(scroll down for "Table Logging") - and FirePHP -
http://www.firephp.org/HQ/Use.htm
(scroll down for "Logging Messages / Tables") - have support to log tabular information. We should just go ahead and add this support to the console object directly, since it would be useful in other contexts. Thoughts: - method: console.table(array2D) - array2D is expected to be an array of arrays (or array-like things) - top-level array is the list of rows - table size should be #rows x max(length of 2nd level arrays) - first row are the column headers - initially just use toString() on the cell elements, maybe expand to full object later - alternate row coloring, like other tables in WI - left-align strings, right-align other primitives - column sorting (prolly add later) - allow collapsing to just 1st row (column headings)
Attachments
partial patch
(14.36 KB, patch)
2010-06-07 15:53 PDT
,
Patrick Mueller
no flags
Details
Formatted Diff
Diff
partial patch - 2010-06-14-a
(23.79 KB, patch)
2010-06-14 12:18 PDT
,
Patrick Mueller
no flags
Details
Formatted Diff
Diff
screen shot showing some tables
(163.93 KB, image/jpeg)
2010-06-16 13:59 PDT
,
Patrick Mueller
no flags
Details
proposed patch
(49.98 KB, patch)
2010-06-16 16:46 PDT
,
Patrick Mueller
joepeck
: review-
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Pavel Feldman
Comment 1
2010-05-06 11:59:30 PDT
I wonder if we should first see it implemented in wildfire first and then generalize into a reusable component if necessary. Rationale: there are multiple ways wildfire could integrate into inspector ui. We could either - provide a separate panel, or - expose console.table as suggested in the bug (except for it should be json with no toString that is going to cross js worlds) or - expose addToConsole(element) to give greater flexibility I am actually thinking that the last option would make most sense to the plugin implementors.
Patrick Mueller
Comment 2
2010-05-06 12:54:12 PDT
I've pinged John J Barton of FireBug about this as well, to see if there was any interest from them in a new console method.
Pavel Feldman
Comment 3
2010-05-06 12:58:20 PDT
(In reply to
comment #2
)
> I've pinged John J Barton of FireBug about this as well, to see if there was > any interest from them in a new console method.
Oh, so you were talking about the real console api method. I was confused with the firephp reference. I wonder how it is supposed to help with the firephp and such given that it would be exposed on the inspected page side though...
Patrick Mueller
Comment 4
2010-05-06 14:58:42 PDT
(In reply to
comment #3
)
> Oh, so you were talking about the real console api method. I was confused with > the firephp reference. I wonder how it is supposed to help with the firephp and > such given that it would be exposed on the inspected page side though...
My assumption for the FirePHP folks is that they will need to implement this functionality themselves, in code they include in WI patches, or a WI extension (once we have an extension capability). Why let them have all the fun? We just add table as a new ConsoleMessage.MessageType, and implement the code for it in ConsoleView.js. The FirePHP folks, and anyone else, will then have easy access to it if their code is already included in WI, and we can expose "console" as an API in the extension framework (once we have one). The last bit, of hooking this up to the console object that people use today for logging, is trivial.
johnjbarton
Comment 5
2010-05-21 15:57:32 PDT
(In reply to
comment #2
)
> I've pinged John J Barton of FireBug about this as well, to see if there was any interest from them in a new console method.
Our implementation is in Firebug 1.6a11, describe here:
http://blog.getfirebug.com/2010/05/21/firebug-1-6a11
Thanks for pulling this together Patrick!
Patrick Mueller
Comment 6
2010-05-28 05:37:16 PDT
I'm starting to look into this one.
Jan Honza Odvarko
Comment 7
2010-05-28 05:49:25 PDT
One addition. It could be quite useful if there is a way how to provide the header information without modifying the original data being logged using console.table(); Firebug implementation now supports following usage scenarios: - console.table(array2D) // as proposed. - console.table("My table", array2d); - console.table("My tables", array2d-1, array2d-2); Since there is a support for multiple arguments/tables (to be more inline with the other console methods), it could be painful to add another argument that contains the header info. Instead I have been thinking about formatted string provided as part of the optional first message parameter. Here are some examples: - console.table("My table %Col1 %Col2 %Col3", array2d); In this case there would be three columns "Col1", "Col2" and "Col3" automatically created - console.table("My table %Col1 %Col2 %Col3", array2d-1, array2d-2); If there is more arguments/tables they would share the same column set. If the formatted cols are not provided, the first row in the data would be expected to define the header, just like originally proposed. This could be perhaps too much but in case of more tables the formatted string could specify different sets of headers, something like as follows: - console.table("My table [%Col1 %Col2 %Col3] [%Col1 %Col2]", array2d-3cols, array2d-2cols); Perhaps, the syntax could be yet improved. What do you think? Honza
Patrick Mueller
Comment 8
2010-05-28 07:04:32 PDT
(In reply to
comment #7
)
> What do you think?
I'm not a big fan of overridden parameters, and it looks like this is getting a bit out of control! :-) How about we get rid of the ability to output multiple tables - you can already get nesting/grouping via group()/endGroup(). And then take an optional 2nd parameter which is an options object, with a property per option. So you could do the column headers via something like this: console.table(array2D, { columns: ["first", "second", "third"]}) maybe support row headers: console.table(array2D, { rows: ["first row", "second row", "third row"]}) leaves lots of space for additional options - table width (100% or sized-to-fit), cellpadding, etc
Patrick Mueller
Comment 9
2010-05-28 08:44:30 PDT
In addition to arrays of arrays, I see FireBug is also supporting arrays of objects, and then presumably iterating over the properties to obtain table cell values. Sounds kinda useful. Not sure I want to get too fancy here - I think I'd just iterate over the properties with a [for (var key in rowObject)] and assign the row cells one after another. Not try to line up property names across multiple rows, for instance. Another flavor of that would be to accept just an object instead of an array2D, and the create a two column table from that - one row for each property, with property name/value being the row cells.
Jan Honza Odvarko
Comment 10
2010-05-31 03:19:16 PDT
(In reply to
comment #8
)
> (In reply to
comment #7
) > > What do you think? > > I'm not a big fan of overridden parameters, and it looks like this is getting a bit out of control! :-) > > How about we get rid of the ability to output multiple tables - you can already get nesting/grouping via group()/endGroup(). > > And then take an optional 2nd parameter which is an options object, with a property per option. So you could do the column headers via something like this: > > console.table(array2D, { columns: ["first", "second", "third"]}) > > maybe support row headers: > > console.table(array2D, { rows: ["first row", "second row", "third row"]}) > > leaves lots of space for additional options - table width (100% or sized-to-fit), cellpadding, etc
OK, you are right, let's specify the column header via an additional parameter and avoid support for multiple arguments. So, using your example, following would log a table with three provided columns: console.table(array2D, { columns: ["first", "second", "third"]}); And the following example would log a table, where columns are dynamically gathered from the first row of provided data (so header is expected to be part of the data): console.table(array2D); What I still like is a way to optionally provide a label for the log i.e.: console.table("My table", array2D); I tend to believe that this will be actually the most useful scenario. The user just logs a table (collapsed by default so, it doesn't take so much space in the console) and the only visible thing is the label (with a toggle button). In case where the title is not specified the table is directly visible, which can be also useful (mainly if the table is rather small). Other option is to have the 'label' parameter mandatory, but then we loose the simplicity in case the user wants to write just: console.table(array2d); Would this work for you? --- As far as the options object is concerned, do you think the user could also specify sort type for a column? Firebug is now using either alphabetical or numerical sort that dynamically applies according to the actual data type in the second row (first is the header now). E.g. something like: console.table(array2D, { columns: [{label: "first", sort: "alpha"}, "second", "third"]}); - the first column would explicitly use alphabetical sorting regardless of actual data type in the column. Honza
Jan Honza Odvarko
Comment 11
2010-05-31 03:26:31 PDT
(In reply to
comment #9
)
> In addition to arrays of arrays, I see FireBug is also supporting arrays of > objects, and then presumably iterating over the properties to obtain table > cell values. Sounds kinda useful.
I think the motivation here is to support array of objects (the same instances so the same properties), where each displayed row represents one instance.
> Not sure I want to get too fancy here - > I think I'd just iterate over the properties with a [for (var key in > rowObject)] and assign the row cells one after another. Not try to line > up property names across multiple rows, for instance.
Yes agree.
> Another flavor of that would be to accept just an object instead of an > array2D, and the create a two column table from that - one row for each > property, with property name/value being the row cells.
Also agree. Honza
Timothy Hatcher
Comment 12
2010-05-31 05:48:38 PDT
(In reply to
comment #11
)
> (In reply to
comment #9
) > > In addition to arrays of arrays, I see FireBug is also supporting arrays of > > objects, and then presumably iterating over the properties to obtain table > > cell values. Sounds kinda useful. > I think the motivation here is to support array of objects (the same instances so the same properties), where each displayed row represents one instance.
I think both should be supported, arrays are just objects with numeric property names (at the basic level).
> > Not sure I want to get too fancy here - > > I think I'd just iterate over the properties with a [for (var key in > > rowObject)] and assign the row cells one after another. Not try to line > > up property names across multiple rows, for instance. > Yes agree.
I adon't agree. You should line them up. The property order of the first row can be used for the other rows. You can't assume for..in will return the same order for all the objects, since the order it uses is the order the properies are added. And some of the objects might have been made differently. It isn't that hard or fancy to do this. Do it right.
> > Another flavor of that would be to accept just an object instead of an > > array2D, and the create a two column table from that - one row for each > > property, with property name/value being the row cells. > Also agree.
Thinkng about this at a higher level, like what I said about arrays just being objects with numeric properties. That would apply here too. Iterate over the object, one row per property and the value, would be shown as columns, one column per property of the value. Think about it. Then you have automatic labels for the rows and the columns. Just if the objects are arrays the labels are numbers (and thats when you would pass in seperate labels.) I'm not a fan of the options argument. If it just takes columns, just make it be a columns argument. You can add more arguments later if you need them.
Pavel Feldman
Comment 13
2010-05-31 05:57:13 PDT
Jumping in with a small comment: I rarely operate matrices (array2Ds). But I use arrays of objects a lot. Having a convenient way of dumping arrays of objects in a user-friendly manner would be handy. So I would expect console.table to be used for arrays of objects in 99% of cases. Hence, I would form an API as: console.table(objectsArray, propertyName1, propertyName2, propertyName3, ...) or console.table(objectsArray, [propertyName1, propertyName2, propertyName3) providing no property names should dump all of the own properties (united by or, not by and).
Timothy Hatcher
Comment 14
2010-05-31 06:04:06 PDT
(In reply to
comment #13
)
> Jumping in with a small comment: I rarely operate matrices (array2Ds). But I use arrays of objects a lot. Having a convenient way of dumping arrays of objects in a user-friendly manner would be handy. So I would expect console.table to be used for arrays of objects in 99% of cases. Hence, I would form an API as: > > console.table(objectsArray, propertyName1, propertyName2, propertyName3, ...) or > console.table(objectsArray, [propertyName1, propertyName2, propertyName3) > > providing no property names should dump all of the own properties (united by or, not by and).
I agree, being able to limit what properties are shown is needed, otherwise you could never use this to show DOM objects (because they have tons of properties.) Again, arrays are just objects with numeric properties…
Jan Honza Odvarko
Comment 15
2010-05-31 07:10:35 PDT
> I think both should be supported, arrays are just objects with numeric > property names (at the basic level).
Sorry, I didn't express myself clearly, I also think both should be supported. So just to summarize all comments, to make sure we are all on the same page. 1) 2d array var object = [[1,2,3],[4,5,6]]; console.table(object); 0 1 2 ------- 1 2 3 4 5 6 Columns can be customized using 'columns' argument. console.table(object, [0, 2]); 0 2 ---- 1 3 4 6 2) Simple object: var object = {a: 1, b: 2, c: 3} console.table(object); Prop Value ---------- a 1 b 2 c 3 3) List of objects: var object = [{name: "Mike", age: 30},{name: "John", age: 5}]; console.table(object); name age -------- Mike 30 John 5 4) Complex object: var object = {father: {name: "Mike", age: 30}, son: {name: "John", age: 5}}; console.table(object); name age -------- Mike 30 John 5 If row headers are supported it would look like as follows: name age -------- father | Mike 30 son | John 5 5) Customizing displayed properties var object = {father: {name: "Mike", age: 30}, son: {name: "John", age: 5}}; console.table(object, ["name"]); name ---- Mike John 6) Expandable log var object = {father: {name: "Mike", age: 30}, son: {name: "John", age: 5}}; console.table("My Family", object); + My Family The user needs to expand to see the actual data. 7) Since the 'columns' argument specifies, which properties/columns should be displayed, a custom column header (e.g. custom labels) would have to be specified as another additional parameter. var object = {father: {name: "Mike", age: 30}, son: {name: "John", age: 5}}; console.table(object, ["name"], ["Just Names"]); Just Names ---------- Mike John Are the examples correct? Comments: - The first data row (ie. the first object in the provided array, or the first array in the provided array of the first found property in the provided object) is used as the template and the others are expected to be the same. - I am not big fan of the header being part of the data structure, but I am open here. - Not sure if customizing sort-type is interesting for anyone. Honza
Timothy Hatcher
Comment 16
2010-05-31 08:35:11 PDT
(In reply to
comment #15
)
> If row headers are supported it would look like as follows: > > name age > -------- > father | Mike 30 > son | John 5
I think row headers for arrays would be useful too, 0, 1, 2, etc.
> 6) Expandable log > var object = {father: {name: "Mike", age: 30}, son: {name: "John", age: 5}}; > console.table("My Family", object); > > + My Family > > The user needs to expand to see the actual data.
I agree with Patrick, it is weird to treat the parameters differently based on type/order. I think title is a good idea, and agree it is optional. And more likely to be used than specifying columns and headers. So here is the parameter order I would expect. console.table(data[[[, title], columns], headers])
> - The first data row (ie. the first object in the provided array, or the first array in the provided array of the first found property in the provided object) is used as the template and the others are expected to be the same.
There was Pavels idea of adding the columns together for mixed type objects. So given: var people = [{name: "John", company: "Acme"}, {name: "Amy", email: "
amy@foo.com
"}, {name: "Joe", company: "Acme", phone:"555-555-5555"}]; name company email phone ------------------------------------ John Acme -- -- Amy --
amy@foo.com
-- Joe Acme -- 555-555-5555 That is useful for sparse objects, when you have missing data.
Timothy Hatcher
Comment 17
2010-05-31 08:41:43 PDT
Should the headers param be an object instead of an array? Say you skip the columns param (pass null) and just use everything the objects have. You then wont know the property order for rthe headers. So specifying them as an object make the order not matter, and lets you skip some. var data = [{name: "Mike", age: 30}, {name: "John", age: 5}]; console.table(data, null, null, {age: "How Old?"}) name How Old? ------------ Mike 30 John 5
Pavel Feldman
Comment 18
2010-05-31 09:19:03 PDT
> var data = [{name: "Mike", age: 30}, {name: "John", age: 5}]; > console.table(data, null, null, {age: "How Old?"}) >
I'd keep it simple - 'age' is already there, so you know what it means. Unless you are willing to export the data, but I thought this is for logging, not exporting...
Jan Honza Odvarko
Comment 19
2010-06-01 03:30:27 PDT
(In reply to
comment #16
)
> (In reply to
comment #15
) > > If row headers are supported it would look like as follows: > > > > name age > > -------- > > father | Mike 30 > > son | John 5 > > I think row headers for arrays would be useful too, 0, 1, 2, etc.
Yes
> > 6) Expandable log > > var object = {father: {name: "Mike", age: 30}, son: {name: "John", age: 5}}; > > console.table("My Family", object); > > > > + My Family > > > > The user needs to expand to see the actual data. > > I agree with Patrick, it is weird to treat the parameters differently based on type/order. I think title is a good idea, and agree it is optional. And more likely to be used than specifying columns and headers. So here is the parameter order I would expect. > > console.table(data[[[, title], columns], headers])
OK, agree, you convinced me.
> > - The first data row (ie. the first object in the provided array, or the first array in the provided array of the first found property in the provided object) is used as the template and the others are expected to be the same. > > There was Pavels idea of adding the columns together for mixed type objects. > > So given: > > var people = [{name: "John", company: "Acme"}, {name: "Amy", email: "
amy@foo.com
"}, {name: "Joe", company: "Acme", phone:"555-555-5555"}]; > > name company email phone > ------------------------------------ > John Acme -- -- > Amy --
amy@foo.com
-- > Joe Acme -- 555-555-5555 > > That is useful for sparse objects, when you have missing data.
Sound good to me (event if it requires more analysis of provided data before UI generation). Honza
Jan Honza Odvarko
Comment 20
2010-06-01 03:38:28 PDT
(In reply to
comment #17
)
> Should the headers param be an object instead of an array? > > Say you skip the columns param (pass null) and just use everything the objects have. You then wont know the property order for rthe headers. > > So specifying them as an object make the order not matter, and lets you skip some. > > var data = [{name: "Mike", age: 30}, {name: "John", age: 5}]; > console.table(data, null, null, {age: "How Old?"}) > > name How Old? > ------------ > Mike 30 > John 5
In case the headers argument would be an object, we can merge columns and headers into one: var data = [{name: "Mike", age: 30}, {name: "John", age: 5}]; console.table(data, "My Family", {prop: "age", label: "How Old?"}) How Old? -------- 30 5 I guess Patrick would like it and it's also ok with me. Honza
Pavel Feldman
Comment 21
2010-06-01 04:57:22 PDT
> var data = [{name: "Mike", age: 30}, {name: "John", age: 5}]; > console.table(data, "My Family", {prop: "age", label: "How Old?"})
How do I define several properties? Is this array or vararg? This is way too long, I'd appreciate a) console.table(data, ["age"]); Can we support both (a) and (b) below based on the typeof column specifier? b) console.table(data, "My Family", [{ property: "age", sort: "ascending", label: "How Old?" }]) Also, lets make sure that specifying no columns dumps all properties.
Jan Honza Odvarko
Comment 22
2010-06-01 05:05:12 PDT
(In reply to
comment #21
)
> > var data = [{name: "Mike", age: 30}, {name: "John", age: 5}]; > > console.table(data, "My Family", {prop: "age", label: "How Old?"}) > How do I define several properties? Is this array or vararg?
Sorry, it's an array.
> This is way too long, I'd appreciate > > a) console.table(data, ["age"]); > > Can we support both (a) and (b) below based on the typeof column specifier?
Not sure what you mean by "typeof column specifier"
> b) console.table(data, "My Family", [{ property: "age", sort: "ascending", label: "How Old?" }])
This is exactly what I have in mind.
> Also, lets make sure that specifying no columns dumps all properties.
Yes Honza
Pavel Feldman
Comment 23
2010-06-01 05:28:15 PDT
> > a) console.table(data, ["age"]); > > Can we support both (a) and (b) below based on the typeof column specifier? > Not sure what you mean by "typeof column specifier" > > b) console.table(data, "My Family", [{ property: "age", sort: "ascending",
I mean that if the value specifying the column in array is of type "string", we consider it a name of the property and allow short notation (a). If it is of type "object", we consider it of the form { property: "foo", etc. } and hence allow full notation (b).
Jan Honza Odvarko
Comment 24
2010-06-01 05:32:57 PDT
(In reply to
comment #23
)
> > > a) console.table(data, ["age"]); > > > Can we support both (a) and (b) below based on the typeof column specifier? > > Not sure what you mean by "typeof column specifier" > > > b) console.table(data, "My Family", [{ property: "age", sort: "ascending", > > I mean that if the value specifying the column in array is of type "string", we consider it a name of the property and allow short notation (a). If it is of type "object", we consider it of the form { property: "foo", etc. } and hence allow full notation (b).
I like this. Honza
Timothy Hatcher
Comment 25
2010-06-01 05:46:38 PDT
Looks good to me. Just spell out "property" like Pavel sugested instead of "prop".
Patrick Mueller
Comment 26
2010-06-01 07:37:41 PDT
Wow, thanks for all the ideas folks, great stuff. We should get a complete proposal together somewhere - perhaps a google doc, or webkit wiki page to share or something. (In reply to
comment #15
)
> - I am not big fan of the header being part of the data structure, but I am open here.
I agree. Many of the object cases won't need a header, and it doesn't see quite right to include an extra data row for column labels, and potentially data cells in every row for row labels, to a 2d array. Maybe that was already implied by some of the other comments above.
Patrick Mueller
Comment 27
2010-06-01 07:43:49 PDT
Would it be useful to also support SQLResultSet, SQLResultSetRowList per:
http://dev.w3.org/html5/webdatabase/#sqlresultset
as an additional "table data" source? And of course, something similar for the IndexedDB stuff, which I'm not really familiar with:
http://dev.w3.org/2006/webapi/IndexedDB/
Patrick Mueller
Comment 28
2010-06-01 07:55:07 PDT
A note about the implementation for WebKit. I'm got just a bit of code I'm playing with, just getting to the point of accessing the table data. I'm using the existing model of sending data with the console methods over, which means things cover over as proxies. The implication is that there's a lot of proxies here. Namely: 1 - for the data table itself # of rows - a proxy per row # of cells - a proxy per cell Meaning, at minimum, for a rows x cols table, there are (1) + (rows) + (rows*cols) proxy deferences. If I understand things correctly with the way the proxies work. Dereferencing the additional parameters adds more. The upside of using proxies deeply is that none of the logic or CPU needed for sorting, finding all the "columns", etc, needs to go in the debug target (back end), that code and processing can be done in the debug client (front end). Using all these proxies could, of course, could present some latency; for big tables, for slow connections between debug target and debug client, etc. However, I'm of the mind that "fixing" this is a pre-optimization, and we all know about pre-optimizations :-) Thought I'd mention it though, in case anyone thinks this is a real show-stopper, or if there is an easy way around all the proxy dereferences.
Patrick Mueller
Comment 29
2010-06-01 08:42:42 PDT
(In reply to
comment #15
)
> 6) Expandable log > var object = {father: {name: "Mike", age: 30}, son: {name: "John", age: 5}}; > console.table("My Family", object); > > + My Family > > The user needs to expand to see the actual data.
I'm still not convinced of the need for the title passed as a parameter. I see two primary use cases: - iterative calls in the console REPL You have some data, you just want to dump it. You don't care about a title, and you certainly don't want the data display in a contracted form that you HAVE to expand to see - you just want to see it. - elaborate calls from your scripts You may have some data that you'd like to have really nicely formatted, with headers, grouped, etc, which you encode directly in your script so you don't have to type it all the time. In this case, wrappering your table.console() call(s) with console.group()/console.groupEnd() doesn't seem like such a huge burden to me.
Jan Honza Odvarko
Comment 30
2010-06-01 10:05:06 PDT
(In reply to
comment #29
)
> (In reply to
comment #15
) > > > 6) Expandable log > > var object = {father: {name: "Mike", age: 30}, son: {name: "John", age: 5}}; > > console.table("My Family", object); > > > > + My Family > > > > The user needs to expand to see the actual data. > > I'm still not convinced of the need for the title passed as a parameter. I see two primary use cases: > > - iterative calls in the console REPL > > You have some data, you just want to dump it. You don't care about a title, and you certainly don't want the data display in a contracted form that you HAVE to expand to see - you just want to see it. > > - elaborate calls from your scripts > > You may have some data that you'd like to have really nicely formatted, with headers, grouped, etc, which you encode directly in your script so you don't have to type it all the time. In this case, wrappering your table.console() call(s) with console.group()/console.groupEnd() doesn't seem like such a huge burden to me.
Yes both uses cases are correct. The original point was related to the other console methods that mostly takes a string/label as the first parameter, but looking at the list again we can see that console.dir() and console.dirxml() also doesn't expect a label and so, the developer needs to use them together with console.group() in order to make them expandable. It looks like the console.table() could also fall to this category. So following needs to be written if the label parameter is avoided: console.group("My Table") console.table(object, [columns and headers definition]); console.groupEnd(); So, thinking about this again, it's ok for me if the parameter is not there, it'll be probably easier. Honza
Timothy Hatcher
Comment 31
2010-06-01 14:42:20 PDT
You also don't need a title just to make something expandable. console.dirxml in Web Inspector is collapsed by default and expandable. Table could be the saem, jus tshow the headers or the first few rows and make it expandable to show the rest.
Patrick Mueller
Comment 32
2010-06-01 21:57:09 PDT
(In reply to
comment #15
)
> - Not sure if customizing sort-type is interesting for anyone.
Trying to figure out what the sort property of the elements of the column parameter mean, when the values are "ascending" or "descending". I guess the implication is that the data is sorted based on all the columns which indicate a sort parameter. And then if the column labels were actually sort buttons, like many tabular controls support, do we just throw all that sort information out the window? It would be simpler to not have sort information in the columns parameter elements, so we don't have to sort the data at all. Unless specifically requested by pressing the column label/sort button. Given the use case of "dump some data during a console session", it's unlikely you're going to want to be typing out sort specifications, and sorting by pressing column labels is probably good enough. Given the use case of "elaborate output generated by a script", the script can arrange to do the original sort of the data. Note that if we always prefix a row with a row index (except in the case of data == object), then the original sort order can be provided back to the user with a column label/sort button over that row index. Note the quote from
comment #15
above is actually about sort-type: presumably "alpha" or "numeric"? And no one has mentioned it, so I'm happy to live without it either. Summary: ditch the "sort" property.
Pavel Feldman
Comment 33
2010-06-02 00:13:06 PDT
> Summary: ditch the "sort" property.
Totally fine with me + it can be added later in case we need it. Btw, do we want to pass the console.table information along to the console clients? That way tables make their ways to the browser-specific consoles / system console / layout test dumps. I think we pass logs for dir and dirxml along and dump them in toString format, but I am not sure. Anyways, worth thinking. The reason I bring it up is that sorting option would make sense there - you can't sort raw dump in the system console. This all could be saved for later though.
Patrick Mueller
Comment 34
2010-06-02 03:53:00 PDT
(In reply to
comment #33
)
> > Summary: ditch the "sort" property. > > Btw, do we want to pass the console.table information along to the console clients?
I'm not sure it makes sense for things like layout tests, to have nicely aligned data, or even sorted. But since you mention console in general, it makes me wonder about node.js and friends. It's probably worth mentioning, somewhere - node.js or commonjs ml - that we're looking at this new API to see if anyone has input or issues. I'll make some posts later this morning ...
Patrick Mueller
Comment 35
2010-06-02 04:02:18 PDT
I've made a stab at a stand-alone description of console.table(), along with some examples. I implemented a version of console.table() for the paper, so the examples are "live". Fun to play with. I spent more time on the code than the prose (as usual), so that could use some work. If anyone has changes, additional examples, etc, let me know. If we want to move the doc somewhere more editable, no problem, just let me know where.
http://muellerware.org/papers/console-table.html
Note that I will probably remove the "index" column label over the first column; it bugs me, but seemed appropriate at the time.
Jan Honza Odvarko
Comment 36
2010-06-03 03:16:21 PDT
(In reply to
comment #33
)
> > Summary: ditch the "sort" property. > > Totally fine with me + it can be added later in case we need it. >
Also fine with me. Honza
Patrick Mueller
Comment 37
2010-06-07 15:53:41 PDT
Created
attachment 58091
[details]
partial patch Not ready for review. The patch contains the skeletal bits of the patch. Changes to Console.idl/h/cpp, a test case in manual-tests/inspector/console-table.html, and some code in ConsoleView.js that looks like it might do something useful, but actually doesn't. Figured I'd at least post the skeletal bits. Hoping that the rest of the work to do is to flesh out the rendering code in ConsoleView.js.
Pavel Feldman
Comment 38
2010-06-08 13:11:10 PDT
Comment on
attachment 58091
[details]
partial patch Quick review without getting into too much details. WebCore/inspector/front-end/ConsoleView.js:689 + this.message = this.formattedMessage.textContent; indent WebCore/inspector/front-end/ConsoleView.js:675 + this.formattedMessage = this._format(["%O", args[0]]); this makes me think we will want %T some day for table :) WebCore/inspector/front-end/ConsoleView.js:818 + function getFormatAsRow(rowElement) { returning a function from local function is a bit too heavy to my taste. WebCore/inspector/front-end/ConsoleView.js:808 + for (var i=0; i<array2d.length; i++) { spaces, need more spaces! WebCore/manual-tests/inspector/console-table.html:1 + <script> Now that I've spent so much time on testing harness and layout tests, I am saying no to any manual-test attempt. Please look into the LayoutTests/inspector/console* for the hints on how to automate the process. Even dumping div's text content and testing it against golden is better than manual test! WebCore/page/Console.h:70 + ResultMessageType Is this one actually used?
Patrick Mueller
Comment 39
2010-06-08 13:19:35 PDT
For the automated tests, how about I figure out how to dump the innerHTML of the table (actually, the table's container)? I think this would be good enough to catch regressions.
Patrick Mueller
Comment 40
2010-06-14 12:18:20 PDT
Created
attachment 58685
[details]
partial patch - 2010-06-14-a Still not quite ready for review, but the basic logic is pretty much complete; it creates tables correctly for the included manual tests. Additional tests are needed to handle error cases, and probably a few other cases with different data types passed in. Still needs layout tests, which will replace the manual test case this patch includes.
Patrick Mueller
Comment 41
2010-06-14 12:21:36 PDT
On IRC sometime last week or there abouts, was some chatter about getting "new api" approved by someone, and that includes this new "table" method for console. Presumably, if we add this function, we will have to support it forever, so we want to make sure that we really want to add it. Who do I go about getting approval from? I don't believe there is a process in place for such approvals; feel free to use this feature request as a guinea pig for such a process.
Timothy Hatcher
Comment 42
2010-06-14 13:03:14 PDT
Emailing webkit-dev is the best place to propose new web-facing APIs.
Timothy Hatcher
Comment 43
2010-06-14 13:04:44 PDT
Comment on
attachment 58685
[details]
partial patch - 2010-06-14-a Please use WebInspector.DataGrid for this, not a hand-made table. Data grid already is used in the Inspector for DOM storage and profiles. It supports sorting and everything you should need.
Patrick Mueller
Comment 44
2010-06-15 05:45:36 PDT
(In reply to
comment #43
)
> Please use WebInspector.DataGrid for this, not a hand-made table.
It would be great to be able to reuse this, but I don't think I can in it's current shape. It seems to be designed to work against a fixed width, which doesn't work well for narrow tables (the columns are too wide) nor for wide tables (the columns are too narrow). Attempts to work around this (override the width=100% on the table) this complicate the width calculations between the header and body, making them unaligned. The separation of the header/body into separate tables seems to be to allow the body to scroll through a fixed height, which again, doesn't really work for these tables, and so the header/body separation isn't even needed for this case. I suspect DataGrid can be refactored to handle these use cases, but I don't want to pre-req that to get the first release of table.console() in. Or someone can tell me how to produce tables with it, that look like the ones in the proposal here:
http://muellerware.org/papers/console-table.html
Timothy Hatcher
Comment 45
2010-06-15 07:23:29 PDT
DataGrid works just fine for the query view of databases, which is just like the console. So I know what you are trying to do is possible. I think the "inline" class is what you need. Please try doing this, we don't need another "data grid".
https://trac.webkit.org/browser/trunk/WebCore/inspector/front-end/DatabaseQueryView.js#L146
Patrick Mueller
Comment 46
2010-06-16 13:59:14 PDT
Created
attachment 58926
[details]
screen shot showing some tables Getting close to putting up a proposed patch, thought I'd post a screen shot of some tables in action. These tables are generated from the layout test, which you can also run as a stand-alone .html file to see what the tables look like. Not thrilled with the width=100% on the tables, but that appears to be the price of using DataGrid. Suggest enhancing DataGrid to allow "natural" width as a separate feature request, if anyone else is also not thrilled with the width=100% tables.
Patrick Mueller
Comment 47
2010-06-16 16:46:37 PDT
Created
attachment 58946
[details]
proposed patch Ready for review. Column sorting isn't included in this patch, I figure it can be added later, if required.
Patrick Mueller
Comment 48
2010-06-16 17:11:41 PDT
In addition to a code review needed, this patch adds new API to the window.console object. I believe we want to get such new APIs reviewed by some "new API review" process. I don't know what that entails though.
Timothy Hatcher
Comment 49
2010-06-16 17:43:00 PDT
"Emailing webkit-dev is the best place to propose new web-facing APIs."
Joseph Pecoraro
Comment 50
2010-06-17 10:43:31 PDT
Comment on
attachment 58946
[details]
proposed patch I just took a brief pass over this. Sorry that I only really point out style issues. But, since many will need to be fixed better to catch them early. I have to re-read the comments and your test case to catch up on how console.table is used to do a proper review. And since you are waiting for feedback on webkit-dev, I think this brief review is okay for now =)
> + if (this.headersProxy != null) {
I see you using this != null idiom quite a bit. So, are you trying to handle cases where this is either "undefined" or "null" only, and not 0, "", NaN, false? Typically we would just do "if (this.headersProxy)", or if we really want null then !==. Is there a reason this style check might not work here?
> + // this method kicks off retrieving all the data needed to render the table
Comments should be sentences. Start with a capital, end with a period.
> + if (proxyType === "object") this.headersProxy = null;
Use two lines for these single line ifs. There are a bunch of these.
> + return this._errorOccurred("invalid columns parameter");
These error strings would need a WebInspector.UIString(...).
> + // handle an error message > + _errorOccurred: function(message) > + { > + // for now, do nothing - don't print anything > + // should we print an error message? > + },
For comments line these, its nice to add "FIXME: " at the start. Even if its just a question for review, it makes them easier to spot and search for =). I think a red error message in the console would be nice. Like a "console.error(message)" red message.
> +// dataGrid.autoSizeColumns(5);
Commented out line can be removed. Probably experimenting with a non 100% width table?
> + for (columnName in this.columns) > + if (row[columnName] == null) row[columnName] = "";
This leaks "columnName" out to the global scope. Missing `var`: for (var columnName in this.columns) Also, another "== null" that can be "=== null" in this case.
> + } > + else {
Same line. } else {
> + var callback = function(proxyProperties) {
Typically we use "function callback(...)"
> + self.requests--; > + this.requests++;
Private/internal variables like this we normally put an underscore on. So "this._requests". There are a number of other variables that might be able to change as well. But its not too important.
> + return string.substr(1,string.length-2);
Missing a space after the comma.
> + case WebInspector.ConsoleMessage.MessageType.Table: > + typeString = "Table"; > + break; > + > }
Remove the blank line. (There are a few of these sprinkled in the patch).
Patrick Mueller
Comment 51
2010-06-17 11:27:00 PDT
(In reply to
comment #50
)
> (From update of
attachment 58946
[details]
) > I just took a brief pass over this. Sorry that I only really > point out style issues. But, since many will need to be fixed > better to catch them early.
thx! Comments below for clarification on some of the points.
> > + if (this.headersProxy != null) { > > I see you using this != null idiom quite a bit. So, are you > trying to handle cases where this is either "undefined" or "null" > only, and not 0, "", NaN, false? Typically we would just do > "if (this.headersProxy)", or if we really want null then !==. > Is there a reason this style check might not work here?
I took a pass through ALL of the ==/!= checks to turn them into ===/!== where I could. There are places, and this probably not one of them, where I really need != null or == null - I want to be able to trap null or undefined but NOT 0. Lucked into finding that one with one of the test cases. I suppose I should probably comment on WHY the check is made that way, for those special cases. I suspect there are plenty of cases where I can collapse to just a field check, as you suggest.
> > + return this._errorOccurred("invalid columns parameter"); > > These error strings would need a WebInspector.UIString(...). > > > > + // handle an error message > > + _errorOccurred: function(message) > > + { > > + // for now, do nothing - don't print anything > > + // should we print an error message? > > + }, > > For comments line these, its nice to add "FIXME: " at > the start. Even if its just a question for review, it > makes them easier to spot and search for =). > > I think a red error message in the console would be > nice. Like a "console.error(message)" red message.
I first tried to throw a TypeError for these cases, but that didn't work - the exception never made it back over the other side. Didn't investigate why, I guess no one actually needs to do this. Generating a console message was the other option, but decided to defer doing it for now. There are other odd cases, if you run the tests, like tables with a header but no body. It would probably be useful to generate a message for these also, but wasn't sure.
> > + self.requests--; > > + this.requests++; > > Private/internal variables like this we normally put an underscore > on. So "this._requests". There are a number of other variables > that might be able to change as well. But its not too important.
There are no "API" fields in the class, they're all internal, no code outside the class should ever need to touch anything.
Patrick Mueller
Comment 52
2010-11-29 07:38:35 PST
There was some discussion on IRC on 2010/11/29 about the status for the current patch. Here are some notes. I believe I asked up on webkit IRC in June whether console.table() would need a wider review than just web inspector folks, since it was adding new external API to webkit. This was in reference to some other console functionality which was removed or restricted at the time, having to do with profiling (IIRC). The answer was "yes, new API like this needs a wider review". So I posted the following question:
https://lists.webkit.org/pipermail/webkit-dev/2010-June/013278.html
No responses. In light of no interest, I decided to stop working on the patch, since it wasn't clear it would ever be committed.
Peter Kasting
Comment 53
2010-12-08 16:25:09 PST
(In reply to
comment #52
)
> In light of no interest, I decided to stop working on the patch, since it wasn't clear it would ever be committed.
Frequently we WebKit folks need to be pinged, or you need to find particular domain experts and specifically ask them to comment. I would definitely not assume that this is unlikely to be committed. The opposite is probably true: if you haven't heard objections, it's more likely no one minds terribly. In any case, please do keep trying.
Alex Yaroshevich
Comment 54
2011-03-10 10:17:52 PST
I hope work in progress.) It will be a good thing.
Patrick Mueller
Comment 55
2013-02-20 18:03:50 PST
This functionality seems to have been added in the following bug/patch:
https://bugs.webkit.org/show_bug.cgi?id=109453
Guess we can close this one.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug