| Message ID | DB6PR0701MB2664A0AAD17DD5BC8E69B34BE4E10@DB6PR0701MB2664.eurprd07.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | fix for diagnostic/84034 | expand |
On Thu, 2018-01-25 at 18:53 +0000, Bernd Edlinger wrote: > Hi, > > as PR diagnostic/84034 shows, source files with > dos style line endings can cause a glitch in the > terminal emulation that erases the source line that > is supposed to be shown. > > That happens when the colorizing escape sequences are > printed between the CR and the LF. Apparently the LF is > being ignored and thus the following line overwrites > everything from the last source line. > > > The following patch fixes the visual glitch by handling > a CR '\r' like a TAB '\t' character. > > > Bootstrapped and reg-rested in x86_64-pc-linux-gnu. > OK for trunk? Thanks for working on this. [BEGIN BRAIN-DUMP: Before gcc 6, we handled this case by printing (e.g. gcc 5): /tmp/t.c: In function ‘test’: /tmp/t.c:5:20: warning: suggest parentheses around ‘&&’ within ‘||’ [- Wparentheses] (d && b > e) && ^ where all lines would end with LF, apart from the quoted source line, which would end with CR+LF. From gcc 6 onwards, the lines all end with LF, apart from quoted source lines, which end with: CR + (end-colorization) + LF. What's happening is that layout::print_source_line etc treat the CR as non-whitespace, and prints it. Then layout::print_newline () prints the end-colorization and LF. Presumably we *don't* want to preserve that mixed-ending behavior in our output from gcc 5 and earlier descibed above; that seems like an accident of the old implementation, and doesn't seem useful. Your patch effectively turns the CR before the LF into trailing whitespace, stopping the CR from being printed, turning it into just: (end-colorization) + LF and thus fixing the glitch. END BRAIN-DUMP] The patch is OK. Thanks Dave
2018-01-25 Bernd Edlinger <bernd.edlinger@hotmail.de> PR diagnostic/84034 * diagnostic-show-locus.c (get_line_width_without_trailing_whitespace): Handle CR like TAB. (layout::print_source_line): Likewise. (test_get_line_width_without_trailing_whitespace): Add test cases. Index: gcc/diagnostic-show-locus.c =================================================================== --- gcc/diagnostic-show-locus.c (revision 257048) +++ gcc/diagnostic-show-locus.c (working copy) @@ -639,7 +639,7 @@ get_line_width_without_trailing_whitespace (const while (result > 0) { char ch = line[result - 1]; - if (ch == ' ' || ch == '\t') + if (ch == ' ' || ch == '\t' || ch == '\r') result--; else break; @@ -648,7 +648,8 @@ get_line_width_without_trailing_whitespace (const gcc_assert (result <= line_width); gcc_assert (result == 0 || (line[result - 1] != ' ' - && line[result -1] != '\t')); + && line[result -1] != '\t' + && line[result -1] != '\r')); return result; } @@ -673,9 +674,11 @@ test_get_line_width_without_trailing_whitespace () assert_eq ("", 0); assert_eq (" ", 0); assert_eq ("\t", 0); + assert_eq ("\r", 0); assert_eq ("hello world", 11); assert_eq ("hello world ", 11); assert_eq ("hello world \t\t ", 11); + assert_eq ("hello world\r", 11); } #endif /* #if CHECKING_P */ @@ -1176,8 +1179,8 @@ layout::print_source_line (int row, const char *li else m_colorizer.set_normal_text (); } - char c = *line == '\t' ? ' ' : *line; - if (c == '\0') + char c = *line; + if (c == '\0' || c == '\t' || c == '\r') c = ' '; if (c != ' ') {