-
-
Notifications
You must be signed in to change notification settings - Fork 245
Fix/no char no pos squeeze #174
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@sharkdp thoughts on adding trailing double asterisks for a last line squeeze when position is turned off? Would turn this:
Into this:
EDIT: this is now implemented |
There ended up being a lot of edge cases toggling chars, position, trailing byte padding, and number of panels. hexyl/tests/integration_tests.rs Lines 16 to 28 in fed8d5c
so that this:
can turn into this: More context here: |
Understandable, it will take me a while to get to the conflicts at the moment unfortunately. |
For what it's worth, most of the issues covered by this PR have been fixed by the larger architectural changes. There are no longer any artifacts when lines are printed, and squeezing is properly handled in all cases. I've also standardized error handling to use
If there's anything I missed, let me know. |
Okay. I'm fine with either version. So let's keep what we have right now, unless this double asterisk has some precedent in other apps. Or if there is any other advantage.
@mkatychev Do you think you could extract those changes into a new PR? @sharifhsn Or are these things already covered in your new unit tests for squeezing? |
There are some cases not covered by my tests, like squeeze with incomplete last line. And in general, more testing is a good thing, especially since all my tests were unit, not integration. The only changes would have to be the asterisks when printing without position panel. |
@sharkdp I'll start with the tests in a new branch and see what breaks then. |
fed8d5c
to
fe65792
Compare
Fixes #171
Handles squeeze with
--no-characters
Handles squeeze with
--no-position
Added error handling for
write!
macrosPanels field is now
usize
to simplify comparing against lengthsIf position is hidden, a trailing double asterisk is shown in the last line if it is part of a squeeze: