feat(console): implement console.table()#4690
feat(console): implement console.table()#4690ParthMozarkar wants to merge 7 commits intoboa-dev:mainfrom
Conversation
Test262 conformance changes
Tested main commit: |
core/runtime/src/console/mod.rs
Outdated
| /// | ||
| /// [spec]: https://console.spec.whatwg.org/#table | ||
| /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/API/console/table_static | ||
| #[allow(clippy::too_many_lines)] |
There was a problem hiding this comment.
Triggering this lint should've given you a hint that this code is kind of hard to follow 😅
Can you split it in smaller pieces?
There was a problem hiding this comment.
Idea; we could have a crate feature to use the comfy-table crate. Bonus; it supports TTY terminals (setting width accordingly) and non-ANSI (using + and - and | instead of unicode box characters). It would also simplify the code.
There was a problem hiding this comment.
Thanks for the feedback!
You're right the implementation grew larger than it should have.
I'll refactor the logic into smaller helper functions to improve readability and remove the clippy allowance.
Regarding the comfy-table suggestion, would you prefer keeping the implementation internal, or introducing a feature-gated dependency for formatting?
I'll push an update shortly.
There was a problem hiding this comment.
Actually, I'd like to see if all this code can be replaced with comfy-table. If not, we can add it in a separate PR with a feature.
There was a problem hiding this comment.
Thanks for the suggestion!
I'll explore whether the current implementation can be fully replaced using comfy-table. If it works cleanly, I'll update this PR accordingly.
If not, I'll open a separate PR introducing it as a feature as suggested.
|
Hi @jedel1043, I've pushed the refactored implementation addressing the review comments. Changes include: Split the logic into smaller helper functions to improve readability. Removed the clippy::too_many_lines allowance. Improved structure and reduced complexity where possible. Ensured proper error propagation (no expect() in runtime paths). Verified with cargo check, cargo test, and cargo clippy. Please let me know if you'd like any further refinements. Thanks again for the feedback !! |
|
Didn't see you address if you tried implementing everything with comfy-table. Was that done or are you planning on doing it in a later PR? |
I've currently only implemented it for the test output formatting. I am planning on migrating the actual console.table engine logic to use comfy-table entirely in a later PR to keep this PR's scope focused... |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4690 +/- ##
===========================================
+ Coverage 47.24% 57.51% +10.27%
===========================================
Files 476 556 +80
Lines 46892 60817 +13925
===========================================
+ Hits 22154 34981 +12827
- Misses 24738 25836 +1098 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
8729d74 to
31ad673
Compare
Summary
Implements the
console.table()method for theconsolebuiltin as requested in #3806.This adds spec-aligned tabular rendering support to Boa's console implementation.
Implementation Details
tablemethod to theLoggertrait.propertiesargument to allow column filtering.Behavior
undefined, matching browser console behavior.Tests
core/runtime/src/console/tests.rs.Notes
cargo fmt,cargo clippy, and all tests.Fixes #3806