fix(_lp_fill): avoid costly strip_escape on filling sequence #803
Conversation
The previous behavior was to allow escaped sequences in the *fillchars* argument of _lp_fill. This could lead to a very slow call to __lp_strip_escapes if the gap was large. The new behavior is to remove escaped sequences from fillchars, but allow them in the new arguments *fillprefix* and *fillsuffix*. This actually simplifies the _lp_fill function a bit. This patch also removes fixed COLUMNS in theme-preview.sh and fix a spacing problem in the unfold theme.
There was a problem hiding this comment.
This does seem like an improvement. But I don't see the point of fillprefix and fillsufix. Every time you use them, you concat them together with left and right respectively. Why not leave off these new params, and simply document that fillstring will have any formatting removed, so if you want to format it, add that formatting at the end of left (and to reset formatting at the beginning of right if that is what you want).
That's definitely right, it seems obvious in retrospect. |
Yes, I got interrupted while working on it (on a separate commit). Hence why I did not re-request a new review right away :-) |
The previous behavior was to allow escaped sequences in the fillchars argument of _lp_fill. This could lead to a very slow call to __lp_strip_escapes if the gap was large.
The new behavior is to remove escaped sequences from fillchars, but allow them in the new arguments fillprefix and fillsuffix.
This actually simplifies the _lp_fill function a bit.
This patch also removes fixed COLUMNS in theme-preview.sh and fix a spacing problem in the unfold theme.
Ref: this thread on liquidprompt-powerline