143832 2015-04-16 11:20:47 -0700 [CMake] Features list should print dots every other row 2015-04-22 09:41:39 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC All RESOLVED FIXED P2 Trivial --- 143831 1 mcatanzaro mcatanzaro commit-queue mcatanzaro mrobinson oldest_to_newest 1085983 0 mcatanzaro 2015-04-16 11:20:47 -0700 When we print the list of features, dots should be printed every other row, for readability. Like this: -- Enabled features: -- ENABLE_3D_RENDERING ..................... ON -- ENABLE_ACCELERATED_2D_CANVAS ON -- ENABLE_ACCESSIBILITY .................... ON -- ENABLE_API_TESTS ON -- ENABLE_ATTACHMENT_ELEMENT ............... OFF -- ENABLE_BATTERY_STATUS OFF -- ENABLE_CANVAS_PATH ...................... OFF -- ENABLE_CSS3_TEXT OFF I broke this in r182658; the dots are being printed on seemingly-random rows based on the list of all options including hidden options, rather than the list of only printed options. 1085989 1 250932 mcatanzaro 2015-04-16 11:28:20 -0700 Created attachment 250932 Patch 1085991 2 mrobinson 2015-04-16 11:33:23 -0700 Do you want a review for this? 1086001 3 mcatanzaro 2015-04-16 11:55:21 -0700 Yes, but this patch probably won't apply on master. I'll mark it r? after bug #143831 is resolved. 1087385 4 250932 mrobinson 2015-04-22 08:30:24 -0700 Comment on attachment 250932 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250932&action=review > Source/cmake/WebKitFeatures.cmake:214 > if (${_WEBKIT_AVAILABLE_OPTIONS_IS_PUBLIC_${_name}}) Is there some reason this conditional cannot move up to line 199? I don't quite understand why we would build the message if we aren't going to show it. 1087386 5 mcatanzaro 2015-04-22 08:36:16 -0700 I considered moving it up to line 199. My first instinct was to do this: foreach (_name ${_WEBKIT_AVAILABLE_OPTIONS}) if (NOT ${_WEBKIT_AVAILABLE_OPTIONS_IS_PUBLIC_${_name}}) continue () endif () # ... endforeach () But the continue statement would require some newer version of cmake, which doesn't seem worth it for such a small convenience. The alternative is to do this: foreach (_name ${_WEBKIT_AVAILABLE_OPTIONS}) if (${_WEBKIT_AVAILABLE_OPTIONS_IS_PUBLIC_${_name}}) # ... endif () endforeach () But then that adds an extra layer of indentation to the entire outer loop. I had a slight preference to doing the extra work in order to avoid that, since I figure this will add almost no time, but maybe that was silly. Whichever you prefer is fine by me! 1087387 6 mrobinson 2015-04-22 08:38:35 -0700 I understand what you mean about the indent. I have a slight preference for keeping everything inside the indented loop, simply because I found it a bit confusing at first. 1087389 7 250932 mrobinson 2015-04-22 08:40:54 -0700 Comment on attachment 250932 Patch Maybe just modify the loop while landing. I think it's probably fine either way, but I believe the version where all of the logic is guarded by the conditional is a bit less confusing. 1087390 8 251319 mcatanzaro 2015-04-22 08:46:41 -0700 Created attachment 251319 Patch 1087420 9 251319 commit-queue 2015-04-22 09:41:35 -0700 Comment on attachment 251319 Patch Clearing flags on attachment: 251319 Committed r183109: <http://trac.webkit.org/changeset/183109> 1087421 10 commit-queue 2015-04-22 09:41:39 -0700 All reviewed patches have been landed. Closing bug. 250932 2015-04-16 11:28:20 -0700 2015-04-22 08:46:38 -0700 Patch bug-143832-20150416132722.patch text/plain 2656 mcatanzaro U3VidmVyc2lvbiBSZXZpc2lvbjogMTgyODkxCmRpZmYgLS1naXQgYS9Tb3VyY2UvY21ha2UvV2Vi S2l0RmVhdHVyZXMuY21ha2UgYi9Tb3VyY2UvY21ha2UvV2ViS2l0RmVhdHVyZXMuY21ha2UKaW5k ZXggMDhlYjkzZmY2YWY0NDgwNWMwM2I4NzIxMjVjYzgyYzExNGI0Yzc3YS4uYTdlMDMzOTA5NzUw MGY4MWZlMjQ5YjI1ZTlhM2NhZWI5MTA5YTIxMCAxMDA2NDQKLS0tIGEvU291cmNlL2NtYWtlL1dl YktpdEZlYXR1cmVzLmNtYWtlCisrKyBiL1NvdXJjZS9jbWFrZS9XZWJLaXRGZWF0dXJlcy5jbWFr ZQpAQCAtMTk0LDI5ICsxOTQsMjYgQEAgZW5kbWFjcm8oKQogbWFjcm8oUFJJTlRfV0VCS0lUX09Q VElPTlMpCiAgICAgbWVzc2FnZShTVEFUVVMgIkVuYWJsZWQgZmVhdHVyZXM6IikKIAotICAgIHNl dChfU0hPVUxEX1BSSU5UX1BPSU5UUyBPRkYpCisgICAgc2V0KF9TSE9VTERfUFJJTlRfRE9UUyBP TikKICAgICBmb3JlYWNoIChfbmFtZSAke19XRUJLSVRfQVZBSUxBQkxFX09QVElPTlN9KQogICAg ICAgICBzdHJpbmcoTEVOR1RIICR7X25hbWV9IF9OQU1FX0xFTkdUSCkKIAogICAgICAgICBzZXQo X01FU1NBR0UgIiAke19uYW1lfSAiKQogCiAgICAgICAgICMgUHJpbnQgZG90cyBvbiBldmVyeSBv dGhlciByb3csIGZvciByZWFkYWJpbGl0eS4KLSAgICAgICAgaWYgKF9TSE9VTERfUFJJTlRfRE9U UykKLSAgICAgICAgICAgIGZvcmVhY2ggKElHTk9SRSBSQU5HRSAke19OQU1FX0xFTkdUSH0gJHtf TUFYX0ZFQVRVUkVfTEVOR1RIfSkKLSAgICAgICAgICAgICAgICBzZXQoX01FU1NBR0UgIiR7X01F U1NBR0V9ICIpCi0gICAgICAgICAgICBlbmRmb3JlYWNoICgpCi0gICAgICAgICAgICBzZXQoX1NI T1VMRF9QUklOVF9ET1RTIE9GRikKLSAgICAgICAgZWxzZSAoKQotICAgICAgICAgICAgZm9yZWFj aCAoSUdOT1JFIFJBTkdFICR7X05BTUVfTEVOR1RIfSAke19NQVhfRkVBVFVSRV9MRU5HVEh9KQor ICAgICAgICBmb3JlYWNoIChJR05PUkUgUkFOR0UgJHtfTkFNRV9MRU5HVEh9ICR7X01BWF9GRUFU VVJFX0xFTkdUSH0pCisgICAgICAgICAgICBpZiAoJHtfU0hPVUxEX1BSSU5UX0RPVFN9KQogICAg ICAgICAgICAgICAgIHNldChfTUVTU0FHRSAiJHtfTUVTU0FHRX0uIikKLSAgICAgICAgICAgIGVu ZGZvcmVhY2ggKCkKLSAgICAgICAgICAgIHNldChfU0hPVUxEX1BSSU5UX0RPVFMgT04pCi0gICAg ICAgIGVuZGlmICgpCisgICAgICAgICAgICBlbHNlICgpCisgICAgICAgICAgICAgICAgc2V0KF9N RVNTQUdFICIke19NRVNTQUdFfSAiKQorICAgICAgICAgICAgZW5kaWYgKCkKKyAgICAgICAgZW5k Zm9yZWFjaCAoKQogCiAgICAgICAgIHNldChfTUVTU0FHRSAiJHtfTUVTU0FHRX0gJHske19uYW1l fX0iKQogCiAgICAgICAgIGlmICgke19XRUJLSVRfQVZBSUxBQkxFX09QVElPTlNfSVNfUFVCTElD XyR7X25hbWV9fSkKICAgICAgICAgICAgIG1lc3NhZ2UoU1RBVFVTICIke19NRVNTQUdFfSIpCisg ICAgICAgICAgICBzZXQoX1NIT1VMRF9QUklOVF9ET1RTIChOT1QgJHtfU0hPVUxEX1BSSU5UX0RP VFN9KSkKICAgICAgICAgZW5kaWYgKCkKICAgICBlbmRmb3JlYWNoICgpCiBlbmRtYWNybygpCmRp ZmYgLS1naXQgYS9DaGFuZ2VMb2cgYi9DaGFuZ2VMb2cKaW5kZXggZjRhYWUxNGQ2NDNlYWIxMzI2 MzBiZDcyZDQ5ZjIwOTRjYjNhODJmOS4uZTk3YTgzOTBmZmFkOGJmOGQyNDc0OGZlMGExNzA5YjBj YzQ2OGI3MCAxMDA2NDQKLS0tIGEvQ2hhbmdlTG9nCisrKyBiL0NoYW5nZUxvZwpAQCAtMSw1ICsx LDIwIEBACiAyMDE1LTA0LTE2ICBNaWNoYWVsIENhdGFuemFybyAgPG1jYXRhbnphcm9AaWdhbGlh LmNvbT4KIAorICAgICAgICBbQ01ha2VdIEZlYXR1cmVzIGxpc3Qgc2hvdWxkIHByaW50IGRvdHMg ZXZlcnkgb3RoZXIgcm93CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD0xNDM4MzIKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKwor ICAgICAgICBEaXNjb3VudCBwcml2YXRlIG9wdGlvbnMgd2hlbiBkZXRlcm1pbmluZyB3aGV0aGVy IHRvIHByaW50IGRvdHMgb24gYSBnaXZlbiByb3cgb2YgdGhlCisgICAgICAgIGZlYXR1cmVzIGxp c3QuIEFsc28sIHNpbXBsaWZ5IHRoZSBsb2dpYyBieSB1c2luZyBvbmx5IG9uZSBmb3IgbG9vcCwg YW5kIGZpeCBhIGNvdXBsZSBlcnJvcnMKKyAgICAgICAgKGludmVydGVkIHVzZSBvZiBTSE9VTERf UFJJTlRfRE9UUywgb25lIHVzZSBvZiB0aGUgcmVuYW1lZCB2YXJpYWJsZSBTSE9VTERfUFJJTlRf UE9JTlRTIHRoYXQKKyAgICAgICAgd2Fzbid0IG5vdGljZWQgYmVjYXVzZSBpdCBvbmx5IGFmZmVj dHMgdGhlIGZpcnN0IGxpbmUsIGFuZCB1c2Ugb2YgdGhlIHZhcmlhYmxlIG5hbWUgYXMgYQorICAg ICAgICBzdHJpbmcgaW4gYSBjb25kaXRpb25hbC4pCisKKyAgICAgICAgKiBTb3VyY2UvY21ha2Uv V2ViS2l0RmVhdHVyZXMuY21ha2U6CisKKzIwMTUtMDQtMTYgIE1pY2hhZWwgQ2F0YW56YXJvICA8 bWNhdGFuemFyb0BpZ2FsaWEuY29tPgorCiAgICAgICAgIFtDTWFrZV0gUmVxdWlyZSBzcGVjaWZ5 aW5nIHZpc2liaWxpdHkgb2YgV2ViS2l0IG9wdGlvbnMKICAgICAgICAgaHR0cHM6Ly9idWdzLndl YmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE0MzgzMQogCg== 251319 2015-04-22 08:46:41 -0700 2015-04-22 09:41:35 -0700 Patch bug-143832-20150422104537.patch text/plain 2885 mcatanzaro U3VidmVyc2lvbiBSZXZpc2lvbjogMTgzMTAyCmRpZmYgLS1naXQgYS9Tb3VyY2UvY21ha2UvV2Vi S2l0RmVhdHVyZXMuY21ha2UgYi9Tb3VyY2UvY21ha2UvV2ViS2l0RmVhdHVyZXMuY21ha2UKaW5k ZXggZWE4NzgzODUxZGYyOTA4ZTBlZDgyZTE1YjY5NmRiYTkzZWYxZjAyMC4uNTRmZTMyNzY2ZWVj MTRmZGM5MjdkMDgzZTUyYzhmZDYwNTJhMmEzZiAxMDA2NDQKLS0tIGEvU291cmNlL2NtYWtlL1dl YktpdEZlYXR1cmVzLmNtYWtlCisrKyBiL1NvdXJjZS9jbWFrZS9XZWJLaXRGZWF0dXJlcy5jbWFr ZQpAQCAtMTk1LDI4ICsxOTUsMjQgQEAgZW5kbWFjcm8oKQogbWFjcm8oUFJJTlRfV0VCS0lUX09Q VElPTlMpCiAgICAgbWVzc2FnZShTVEFUVVMgIkVuYWJsZWQgZmVhdHVyZXM6IikKIAotICAgIHNl dChfU0hPVUxEX1BSSU5UX1BPSU5UUyBPRkYpCisgICAgc2V0KF9TSE9VTERfUFJJTlRfRE9UUyBP TikKICAgICBmb3JlYWNoIChfbmFtZSAke19XRUJLSVRfQVZBSUxBQkxFX09QVElPTlN9KQotICAg ICAgICBzdHJpbmcoTEVOR1RIICR7X25hbWV9IF9OQU1FX0xFTkdUSCkKLQotICAgICAgICBzZXQo X01FU1NBR0UgIiAke19uYW1lfSAiKQorICAgICAgICBpZiAoJHtfV0VCS0lUX0FWQUlMQUJMRV9P UFRJT05TX0lTX1BVQkxJQ18ke19uYW1lfX0pCisgICAgICAgICAgICBzdHJpbmcoTEVOR1RIICR7 X25hbWV9IF9OQU1FX0xFTkdUSCkKKyAgICAgICAgICAgIHNldChfTUVTU0FHRSAiICR7X25hbWV9 ICIpCiAKLSAgICAgICAgIyBQcmludCBkb3RzIG9uIGV2ZXJ5IG90aGVyIHJvdywgZm9yIHJlYWRh YmlsaXR5LgotICAgICAgICBpZiAoX1NIT1VMRF9QUklOVF9ET1RTKQorICAgICAgICAgICAgIyBQ cmludCBkb3RzIG9uIGV2ZXJ5IG90aGVyIHJvdywgZm9yIHJlYWRhYmlsaXR5LgogICAgICAgICAg ICAgZm9yZWFjaCAoSUdOT1JFIFJBTkdFICR7X05BTUVfTEVOR1RIfSAke19NQVhfRkVBVFVSRV9M RU5HVEh9KQotICAgICAgICAgICAgICAgIHNldChfTUVTU0FHRSAiJHtfTUVTU0FHRX0gIikKKyAg ICAgICAgICAgICAgICBpZiAoJHtfU0hPVUxEX1BSSU5UX0RPVFN9KQorICAgICAgICAgICAgICAg ICAgICBzZXQoX01FU1NBR0UgIiR7X01FU1NBR0V9LiIpCisgICAgICAgICAgICAgICAgZWxzZSAo KQorICAgICAgICAgICAgICAgICAgICBzZXQoX01FU1NBR0UgIiR7X01FU1NBR0V9ICIpCisgICAg ICAgICAgICAgICAgZW5kaWYgKCkKICAgICAgICAgICAgIGVuZGZvcmVhY2ggKCkKLSAgICAgICAg ICAgIHNldChfU0hPVUxEX1BSSU5UX0RPVFMgT0ZGKQotICAgICAgICBlbHNlICgpCi0gICAgICAg ICAgICBmb3JlYWNoIChJR05PUkUgUkFOR0UgJHtfTkFNRV9MRU5HVEh9ICR7X01BWF9GRUFUVVJF X0xFTkdUSH0pCi0gICAgICAgICAgICAgICAgc2V0KF9NRVNTQUdFICIke19NRVNTQUdFfS4iKQot ICAgICAgICAgICAgZW5kZm9yZWFjaCAoKQotICAgICAgICAgICAgc2V0KF9TSE9VTERfUFJJTlRf RE9UUyBPTikKLSAgICAgICAgZW5kaWYgKCkKIAotICAgICAgICBzZXQoX01FU1NBR0UgIiR7X01F U1NBR0V9ICR7JHtfbmFtZX19IikKKyAgICAgICAgICAgIHNldChfU0hPVUxEX1BSSU5UX0RPVFMg KE5PVCAke19TSE9VTERfUFJJTlRfRE9UU30pKQogCi0gICAgICAgIGlmICgke19XRUJLSVRfQVZB SUxBQkxFX09QVElPTlNfSVNfUFVCTElDXyR7X25hbWV9fSkKKyAgICAgICAgICAgIHNldChfTUVT U0FHRSAiJHtfTUVTU0FHRX0gJHske19uYW1lfX0iKQogICAgICAgICAgICAgbWVzc2FnZShTVEFU VVMgIiR7X01FU1NBR0V9IikKICAgICAgICAgZW5kaWYgKCkKICAgICBlbmRmb3JlYWNoICgpCmRp ZmYgLS1naXQgYS9DaGFuZ2VMb2cgYi9DaGFuZ2VMb2cKaW5kZXggOTQ3ODM1NmYzYmZkZDYyODk2 YTRkZGMwOGQxNTFhM2I2OTg4ODcyMy4uMGRmNjU4OWMxZmMzM2Q4YzkyYTY0NmQzOGUyM2QyYzUy NTA0MTA4OCAxMDA2NDQKLS0tIGEvQ2hhbmdlTG9nCisrKyBiL0NoYW5nZUxvZwpAQCAtMSw1ICsx LDIwIEBACiAyMDE1LTA0LTIyICBNaWNoYWVsIENhdGFuemFybyAgPG1jYXRhbnphcm9AaWdhbGlh LmNvbT4KIAorICAgICAgICBbQ01ha2VdIEZlYXR1cmVzIGxpc3Qgc2hvdWxkIHByaW50IGRvdHMg ZXZlcnkgb3RoZXIgcm93CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD0xNDM4MzIKKworICAgICAgICBSZXZpZXdlZCBieSBNYXJ0aW4gUm9iaW5zb24uCisK KyAgICAgICAgRGlzY291bnQgcHJpdmF0ZSBvcHRpb25zIHdoZW4gZGV0ZXJtaW5pbmcgd2hldGhl ciB0byBwcmludCBkb3RzIG9uIGEgZ2l2ZW4gcm93IG9mIHRoZQorICAgICAgICBmZWF0dXJlcyBs aXN0LiBBbHNvLCBzaW1wbGlmeSB0aGUgbG9naWMgYnkgdXNpbmcgb25seSBvbmUgZm9yIGxvb3As IGFuZCBmaXggYSBjb3VwbGUgZXJyb3JzCisgICAgICAgIChpbnZlcnRlZCB1c2Ugb2YgU0hPVUxE X1BSSU5UX0RPVFMsIG9uZSB1c2Ugb2YgdGhlIHJlbmFtZWQgdmFyaWFibGUgU0hPVUxEX1BSSU5U X1BPSU5UUyB0aGF0CisgICAgICAgIHdhc24ndCBub3RpY2VkIGJlY2F1c2UgaXQgb25seSBhZmZl Y3RzIHRoZSBmaXJzdCBsaW5lLCBhbmQgdXNlIG9mIHRoZSB2YXJpYWJsZSBuYW1lIGFzIGEKKyAg ICAgICAgc3RyaW5nIGluIGEgY29uZGl0aW9uYWwuKQorCisgICAgICAgICogU291cmNlL2NtYWtl L1dlYktpdEZlYXR1cmVzLmNtYWtlOgorCisyMDE1LTA0LTE2ICBNaWNoYWVsIENhdGFuemFybyAg PG1jYXRhbnphcm9AaWdhbGlhLmNvbT4KKwogICAgICAgICBbQ01ha2VdIFJlcXVpcmUgc3BlY2lm eWluZyB2aXNpYmlsaXR5IG9mIFdlYktpdCBvcHRpb25zCiAgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNDM4MzEKIAo=