Refactor print logic #133
Refactor print logic #133
Conversation
| } | ||
|
|
||
| c = string[i]; | ||
| switch (c) { |
There was a problem hiding this comment.
Since there are only two cases, do you think an if statement would make this clearer?
if (c == '\0') {
/*
* A null character will be present prior the
* terminating null character if descriptions is
* enabled.
*/
c = ' ';
col++;
} else if (!isu8cont(c))
col++;
There was a problem hiding this comment.
Totally valid point. However, I plan on fixing output of tabs later
which is currently broken. Then the number of conditions is no longer
binary, therefore the pro-active switch.
|
Hey @mptre, thanks so much for this! It looks, builds, and runs great on my system. I had one small question. Other than that this looks good to merge! @teoljungberg Would you like to confirm that this resolves the flickering issues in tmux on your system? |
|
@calleerlandsson Indeed it does! |
|
Thanks @teoljungberg. |
|
Thank you two for working on this! |
|
I sneaked in a minor cleanup, let me know what you think. Other that |
|
Looks great, @mptre! Could you rebase this on the current master and squash in the mixup commit? |
The match offsets are set to 0 if no match is found. But 0 is also a valid offset. Therefore, make a existing and non-existing match disjoint by using -1 as a indicator for a missing match.
Also, inline counting of ANSI escape sequences for two reasons: - Performing the count while processing the input could cause incorrect behavior if a line is longer than the number of columns and contains ANSI escape sequences at a offset greater than the number of columns. This could be solved by only examining `MIN(columns, choice->length)` bytes, but it could also cause problems explained below. - Simplifies future handling of SIGWINCH since any re-calculation of the number of ANSI escape sequences is no longer needed.
In a attempt to solve #131.
Since printing the query and choices are similar.
- Declarations comes first, in alphabetical order. - Initializers comes second, one per line, in alphabetical order.
|
Done! |
|
Thanks! I'll merge this. |
An attempt to solve #131 turned into a bigger change than expected, for better IMHO. See each commit messages for more info. Would really like to get feedback from all of you.