Let string handle displayed width #8182
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
IlanCosman
reviewed
Jul 29, 2021
|
Also: closes #4012 I believe. |
|
Do we want |
|
I'd say it should probably be on by default for |
|
Okay, this still needs adjustment in E.g. string pad -w 4 ab\rfoohas a maximum width of 3, but we would pad from the beginning, so we influence a run with a width of 2. So we need to add 2 spaces. If we padded to the right, we would need 1 space. |
faho
force-pushed
the
string-pad-escapes
branch
from
1d0bf05
to
1bb74b6
Compare
Uncached, but we don't want to keep this globally, I think? This is useful for doing string pad/length without escapes.
This just changes it so it subtracts escape sequences, according to the current terminal.
Without escapes. The new option is a bit cheesy, but "width" isn't as expressive and requires an argument. Maybe we want "pad" to also require --visible?
Because we are, ultimately, interested in how many cells a string occupies, we *have* to handle carriage return (`\r`) and line feed (`\n`). A carriage return sets the current tally to 0, and only the longest tally is kept. The idea here is that the last position is the same as the last position of the longest string. So: abcdef\r123 ends up looking like 123def which is the same width as abcdef, 6. A line feed meanwhile means we flush the current tally and start a new one. Every line is printed separately, even if it's given as one. That's because, well, counting the width over multiple lines doesn't *help*. As a sidenote: This is necessarily imperfect, because, while we may know the width of the terminal ($COLUMNS), we don't know the current cursor position. So we can only give the width, and the user can then figure something out on their own. But for the common case of figuring out how wide the prompt is, this should do.
faho
force-pushed
the
string-pad-escapes
branch
from
1bb74b6
to
349236f
Compare
|
Alright, let's do the additional pad stuff later. It's probably overly completionist anyway. Merged. |
Good call 👍 Thanks for working on this Fabian 😄 |
This pull request was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This makes
string padpad to the effective width without escape sequences we recognize, and it adds an option--visibletostring lengththat makes it show the visible length in terminal cells (or width, if you prefer).This is, of course, necessarily imperfect. We're not the terminal, we don't know which escape codes it recognizes exactly. But we do handle the most common ones like colors and such.
This also changes in response to $TERM, $fish_emoji_width and $fish_ambiguous_width.
Importantly:
string length --visiblehandles carriage return and line feed in what I hope is a sensible manner. Because counting a string likeas anything other than a width of 6 seems useless to me. The line feed handling may be surprising because it might print multiple lines of output in response to a single argument, but I also don't see how anything else would help.
It would also be possible to make
string length --visiblea separate command instead, but given how similar these operations are I'm not sure that helps. Or we could change the option to--width, but that doesn't include that it also removes escape sequences.Fixes issue #7784.
TODOs: