Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Custom Styling Fix #140

Merged
merged 1 commit into from
Apr 5, 2024
Merged

Custom Styling Fix #140

merged 1 commit into from
Apr 5, 2024

Conversation

dsully
Copy link
Contributor

@dsully dsully commented Apr 4, 2024

This fixes #137. My OCD had to fix typos as well. 😄

  • Fix string width calculation with ANSI escape sequences by using ansi-str instead of console::measure_text_width().
  • Fix typos.
  • Fix compiler warnings in tests/all/property_test.rs

- Fix string width calculation with ANSI escape sequences by using ansi-str instead of console::measure_text_width().
- Fix typos.
- Fix compiler warnings in tests/all/property_test.rs
Copy link

codecov bot commented Apr 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.54%. Comparing base (5df977e) to head (3e8e175).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #140      +/-   ##
==========================================
+ Coverage   96.33%   96.54%   +0.20%     
==========================================
  Files          18       16       -2     
  Lines        1774     1766       -8     
==========================================
- Hits         1709     1705       -4     
+ Misses         65       61       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Nukesor
Copy link
Owner

Nukesor commented Apr 5, 2024

Nice! Renames and code cleanups look good as well :) Thanks!

The only thing I don't really like is that this includes a fairly unknown library, which is basically only used by tabled. tabled is pretty actively maintained by the same author though, so I guess this is somewhat viable.

Also, I would really like to only have one of the dependencies, ansi-str or console, but from what it looks like, ansi-str has no functionality for controlling stdout, which is used in comfy-table's tests.

Might be nice to refactor everything so console is only used as a test dependency though. I like to keep the dependencies users pull as low as possible (reduces build times).

@dsully
Copy link
Contributor Author

dsully commented Apr 5, 2024

I agree - but wanted to get something working short term. console is used for some non-test code as well.

Another alternate short term is I can switch to https://crates.io/crates/strip-ansi-escapes I believe instead of ansi-str

@Nukesor
Copy link
Owner

Nukesor commented Apr 5, 2024

Ah damn. I overlooked some usages.

Nevermind 😓

@Nukesor Nukesor merged commit 4dfab5c into Nukesor:main Apr 5, 2024
18 checks passed
@Nukesor
Copy link
Owner

Nukesor commented Apr 5, 2024

@dsully https://github.com/Nukesor/comfy-table/releases/tag/v7.1.1

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow for OSC / Control Characters
2 participants