19509 2008-06-11 19:49:51 -0700 Database Tables in the Inspector should be sortable 2010-06-18 20:35:14 -0700 1 1 1 Unclassified WebKit Web Inspector (Deprecated) 528+ (Nightly build) All All RESOLVED FIXED P2 Enhancement --- 1 keishi jberlin jberlin timothy tom.devart oldest_to_newest 83081 0 keishi 2008-06-11 19:49:51 -0700 Database tables in the Inspector should be sortable by clicking on the headers. 140701 1 tom.devart 2009-08-17 05:13:03 -0700 It would also be helpful if you could adjust the widths of individual columns, currently you have to adjust the width of the entire window. Any news on if this is being looked at? 229231 2 56774 jberlin 2010-05-21 21:08:00 -0700 Created attachment 56774 Adds the ability to sort a Database Table in the Web Inspector. It is already possible to resize the columns. This patch just adds the ability to sort the table based on the contents of the selected column. Works on both the tables in the Table View and the tables that show up in the Query View. 237684 3 56774 darin 2010-06-13 20:53:15 -0700 Comment on attachment 56774 Adds the ability to sort a Database Table in the Web Inspector. > + var comparison; > + if (isNaN(item1) || isNaN(item2)) > + comparison = item1 < item2 ? -1 : (item1 > item2 ? 1 : 0); > + else { > + // Don't sort numbers alphanumerically. > + var number1 = parseFloat(item1); > + var number2 = parseFloat(item2); > + comparison = number1 < number2 ? -1 : (number1 > number2 ? 1 : 0); > + } I think the comments here are a little strange and the code a bit oblique. I presume the idea here is that if the values are both numeric, we want to compare them as numbers. But I don't see how isNaN accomplishes this. What does that function do exactly? Does it look only at the numeric prefix of a number or will it return false for something like "1a". Also, comparing some pairs with one sort function and other pairs with a different one is going to create trouble for the sort algorithm. Fortunately we fixed this a while back so it couldn't cause a crash in JavaScriptCore by using a tree sort rather than something like qsort that depends on consistent results from sort function. If you wanted the code to behavior like this I think you'd need to explain why isNaN works to check if things are numeric. Even though NaN is "not a number", I think the usage here is unclear. But I think there are some possibly better options: 1) Distinguish numeric columns and non-numeric columns and chose the sort function based on the column type. 2) Write a comparison function that works like kCFCompareNumerically, although I'm not sure that will give results that are good for columns of floating point numbers. 3) Scan the column and sort numerically only if every item in the column s a number. Maybe you can think of something else that will work even better. Also, I would say "Sort numbers based on comparing their values rather than a lexicographical comparison". I think the term "alphanumerically" doesn't describe lexicographical comparison. r=me as is, but I think the comparison function can be improved 240253 4 jberlin 2010-06-18 20:35:02 -0700 (In reply to comment #3) > (From update of attachment 56774 [details]) > > + var comparison; > > + if (isNaN(item1) || isNaN(item2)) > > + comparison = item1 < item2 ? -1 : (item1 > item2 ? 1 : 0); > > + else { > > + // Don't sort numbers alphanumerically. > > + var number1 = parseFloat(item1); > > + var number2 = parseFloat(item2); > > + comparison = number1 < number2 ? -1 : (number1 > number2 ? 1 : 0); > > + } > > I think the comments here are a little strange and the code a bit oblique. I presume the idea here is that if the values are both numeric, we want to compare them as numbers. But I don't see how isNaN accomplishes this. What does that function do exactly? Does it look only at the numeric prefix of a number or will it return false for something like "1a". Also, comparing some pairs with one sort function and other pairs with a different one is going to create trouble for the sort algorithm. Fortunately we fixed this a while back so it couldn't cause a crash in JavaScriptCore by using a tree sort rather than something like qsort that depends on consistent results from sort function. > > If you wanted the code to behavior like this I think you'd need to explain why isNaN works to check if things are numeric. Even though NaN is "not a number", I think the usage here is unclear. I am now using isNaN with the result from Number(item). > > But I think there are some possibly better options: > > 1) Distinguish numeric columns and non-numeric columns and chose the sort function based on the column type. > > 2) Write a comparison function that works like kCFCompareNumerically, although I'm not sure that will give results that are good for columns of floating point numbers. > > 3) Scan the column and sort numerically only if every item in the column s a number. I went with this option. > > Maybe you can think of something else that will work even better. Also, I would say "Sort numbers based on comparing their values rather than a lexicographical comparison". Fixed the comment. > I think the term "alphanumerically" doesn't describe lexicographical comparison. > > r=me as is, but I think the comparison function can be improved Thanks for the review! Committed in r61463: http://trac.webkit.org/changeset/61463 56774 2010-05-21 21:08:00 -0700 2010-06-18 20:35:14 -0700 Adds the ability to sort a Database Table in the Web Inspector. makeDataBaseTablesSortable.patch text/plain 2993 jberlin ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg NTg2MTQyNi4uZmNkNzgyMSAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxOCBAQAorMjAxMC0wNS0yMSAgSmVzc2llIEJlcmxp biAgPGpiZXJsaW5Ad2Via2l0Lm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICBCdWcgMTk1MDkgLSBEYXRhYmFzZSBUYWJsZXMgaW4gdGhlIEluc3Bl Y3RvciBzaG91bGQgYmUgc29ydGFibGUKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcv c2hvd19idWcuY2dpP2lkPTE5NTA5CisKKyAgICAgICAgKiBpbnNwZWN0b3IvZnJvbnQtZW5kL1N0 b3JhZ2VQYW5lbC5qczoKKyAgICAgICAgKFdlYkluc3BlY3Rvci5TdG9yYWdlUGFuZWwucHJvdG90 eXBlLmRhdGFHcmlkRm9yUmVzdWx0KToKKyAgICAgICAgTWFrZSBlYWNoIGNvbHVtbiBpbiB0aGUg RGF0YUdyaWQgc29ydGFibGUuCisgICAgICAgIChXZWJJbnNwZWN0b3IuU3RvcmFnZVBhbmVsLnBy b3RvdHlwZS5fc29ydERhdGFHcmlkLmNvbXBhcmF0b3IpOgorICAgICAgICBTaW5jZSBhbGwgdGhl IGVudHJpZXMgd2lsbCBiZSBzdHJpbmdzLCB0cnkgdG8gZmluZCB0aGUgc3RyaW5ncyB0aGF0IHJl cHJlc2VudCBudW1iZXJzIGFuZCBub3Qgc29ydCB0aGVtIGFscGhhbnVtZXJpY2FsbHkuCisgICAg ICAgIChXZWJJbnNwZWN0b3IuU3RvcmFnZVBhbmVsLnByb3RvdHlwZS5fc29ydERhdGFHcmlkKToK KyAgICAgICAgU29ydCB0aGUgZW50cmllcyBpbiB0aGUgRGF0YUdyaWQgYmFzZWQgb24gdGhlIHNl bGVjdGVkIGNvbHVtbi4KKwogMjAxMC0wNS0yMSAgVmljdG9yaWEgS2lyc3QgIDx2cmtAZ29vZ2xl LmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBEYXZpZCBMZXZpbi4KZGlmZiAtLWdpdCBhL1dl YkNvcmUvaW5zcGVjdG9yL2Zyb250LWVuZC9TdG9yYWdlUGFuZWwuanMgYi9XZWJDb3JlL2luc3Bl Y3Rvci9mcm9udC1lbmQvU3RvcmFnZVBhbmVsLmpzCmluZGV4IGNhMWIyNzYuLjEwMGI0N2IgMTAw NjQ0Ci0tLSBhL1dlYkNvcmUvaW5zcGVjdG9yL2Zyb250LWVuZC9TdG9yYWdlUGFuZWwuanMKKysr IGIvV2ViQ29yZS9pbnNwZWN0b3IvZnJvbnQtZW5kL1N0b3JhZ2VQYW5lbC5qcwpAQCAtMjg5LDYg KzI4OSw3IEBAIFdlYkluc3BlY3Rvci5TdG9yYWdlUGFuZWwucHJvdG90eXBlID0gewogICAgICAg ICAgICAgdmFyIGNvbHVtbiA9IHt9OwogICAgICAgICAgICAgY29sdW1uLndpZHRoID0gY29sdW1u SWRlbnRpZmllci5sZW5ndGg7CiAgICAgICAgICAgICBjb2x1bW4udGl0bGUgPSBjb2x1bW5JZGVu dGlmaWVyOworICAgICAgICAgICAgY29sdW1uLnNvcnRhYmxlID0gdHJ1ZTsKIAogICAgICAgICAg ICAgY29sdW1uc1tjb2x1bW5JZGVudGlmaWVyXSA9IGNvbHVtbjsKICAgICAgICAgICAgICsrbnVt Q29sdW1uczsKQEAgLTMxMyw5ICszMTQsMzkgQEAgV2ViSW5zcGVjdG9yLlN0b3JhZ2VQYW5lbC5w cm90b3R5cGUgPSB7CiAgICAgICAgIGZvciAodmFyIGkgPSAwOyBpIDwgbGVuZ3RoOyArK2kpCiAg ICAgICAgICAgICBkYXRhR3JpZC5hcHBlbmRDaGlsZChub2Rlc1tpXSk7CiAKKyAgICAgICAgZGF0 YUdyaWQuYWRkRXZlbnRMaXN0ZW5lcigic29ydGluZyBjaGFuZ2VkIiwgdGhpcy5fc29ydERhdGFH cmlkLmJpbmQodGhpcywgZGF0YUdyaWQpLCB0aGlzKTsKICAgICAgICAgcmV0dXJuIGRhdGFHcmlk OwogICAgIH0sCiAKKyAgICBfc29ydERhdGFHcmlkOiBmdW5jdGlvbihkYXRhR3JpZCkKKyAgICB7 CisgICAgICAgIHZhciBub2RlcyA9IGRhdGFHcmlkLmNoaWxkcmVuLnNsaWNlKCk7CisgICAgICAg IHZhciBzb3J0Q29sdW1uSWRlbnRpZmllciA9IGRhdGFHcmlkLnNvcnRDb2x1bW5JZGVudGlmaWVy OworICAgICAgICB2YXIgc29ydERpcmVjdGlvbiA9IGRhdGFHcmlkLnNvcnRPcmRlciA9PT0gImFz Y2VuZGluZyIgPyAxIDogLTE7CisKKyAgICAgICAgZnVuY3Rpb24gY29tcGFyYXRvcihkYXRhR3Jp ZE5vZGUxLCBkYXRhR3JpZE5vZGUyKQorICAgICAgICB7CisgICAgICAgICAgICB2YXIgaXRlbTEg PSBkYXRhR3JpZE5vZGUxLmRhdGFbc29ydENvbHVtbklkZW50aWZpZXJdOworICAgICAgICAgICAg dmFyIGl0ZW0yID0gZGF0YUdyaWROb2RlMi5kYXRhW3NvcnRDb2x1bW5JZGVudGlmaWVyXTsKKwor ICAgICAgICAgICAgdmFyIGNvbXBhcmlzb247CisgICAgICAgICAgICBpZiAoaXNOYU4oaXRlbTEp IHx8IGlzTmFOKGl0ZW0yKSkKKyAgICAgICAgICAgICAgICBjb21wYXJpc29uID0gaXRlbTEgPCBp dGVtMiA/IC0xIDogKGl0ZW0xID4gaXRlbTIgPyAxIDogMCk7CisgICAgICAgICAgICBlbHNlIHsK KyAgICAgICAgICAgICAgICAvLyBEb24ndCBzb3J0IG51bWJlcnMgYWxwaGFudW1lcmljYWxseS4K KyAgICAgICAgICAgICAgICB2YXIgbnVtYmVyMSA9IHBhcnNlRmxvYXQoaXRlbTEpOworICAgICAg ICAgICAgICAgIHZhciBudW1iZXIyID0gcGFyc2VGbG9hdChpdGVtMik7CisgICAgICAgICAgICAg ICAgY29tcGFyaXNvbiA9IG51bWJlcjEgPCBudW1iZXIyID8gLTEgOiAobnVtYmVyMSA+IG51bWJl cjIgPyAxIDogMCk7CisgICAgICAgICAgICB9CisgICAgICAgICAgICByZXR1cm4gc29ydERpcmVj dGlvbiAqIGNvbXBhcmlzb247CisgICAgICAgIH0KKworICAgICAgICBub2Rlcy5zb3J0KGNvbXBh cmF0b3IpOworICAgICAgICBkYXRhR3JpZC5yZW1vdmVDaGlsZHJlbigpOworICAgICAgICBmb3Ig KHZhciBpID0gMDsgaSA8IG5vZGVzLmxlbmd0aDsgaSsrKQorICAgICAgICAgICAgZGF0YUdyaWQu YXBwZW5kQ2hpbGQobm9kZXNbaV0pOworICAgIH0sCisKICAgICB1cGRhdGVET01TdG9yYWdlOiBm dW5jdGlvbihzdG9yYWdlSWQpCiAgICAgewogICAgICAgICB2YXIgZG9tU3RvcmFnZSA9IHRoaXMu X2RvbVN0b3JhZ2VGb3JJZChzdG9yYWdlSWQpOwo=