On 10 April 2012 20:52, Gabriel Dos Reis <g...@integrable-solutions.net> wrote: > On Tue, Apr 10, 2012 at 11:46 AM, Manuel López-Ibáñez > <lopeziba...@gmail.com> wrote: >> On 10 April 2012 00:28, Jason Merrill <ja...@redhat.com> wrote: >>> On 04/09/2012 04:01 PM, Manuel López-Ibáńez wrote: >>>> >>>> * It uses the default cutoff as max_width, whatever it is (as >>>> controlled by -fmessage-length). >>>> * It uses the pretty-printer. The text cannot (should not) wrap >>>> because we still print only max_width chars at most. >>> >>> >>> Hmm, I think if pp_line_cutoff is 0 and we're on a terminal, we still want >>> to use COLUMNS to limit how much of the source we print. >> >> Like this? > > There is a novelty in this new version that I don't think we discussed > before: automatic expansion of tabs to 8 hard space characters. That > number should not be hardcoded as there is no reason to believe a tab > character always expands to 8 space characters. You should check > the environment first; if not present the default expansion number should > be a symbolic constant as opposed to a magic number sprinkled all over the > places. I would also encourage you to introduce more abstraction to > reduce the size of diagnostic_show_locus.
There is no novelty, it has been there from the beginning, and now I realize it was a mistake to do the extra work to implement this. The GCS says: "Calculate column numbers assuming that space and all ASCII printing characters have equal width, and assuming tab stops every 8 columns" http://www.gnu.org/prep/standards/standards.html#Errors So, in fact, libcpp is buggy for not implementing this (as can be seen in emacs). If/When libcpp is fixed, the column info will be correct for tabs. And then, one may care about printing tabs as anything different from a single space. But until then, a single space is a good as anything. In fact this is what is done in c-ppoutput.c. As can be seen below, this simplifies a lot the function, so I appreciate the suggestion. If you have suggestions for removing other code, I will be happy to implement them. The less code, the less potential for bugs. Cheers, Manuel. +/* Print the physical source line corresponding to the location of + this diagnostics, and a caret indicating the precise column. */ +void +diagnostic_show_locus (diagnostic_context * context, + const diagnostic_info *diagnostic) +{ + const char *line; + char *buffer; + expanded_location s; + int real_width; + int max_width; + const char *saved_prefix; + int right_margin = 10; + + if (!context->show_caret + || diagnostic->location <= BUILTINS_LOCATION) + return; + + s = expand_location(diagnostic->location); + line = location_get_source_line (s); + if (line == NULL) + return; + + real_width = strlen (line); + + max_width = context->caret_max_width; + if (max_width <= 0) + max_width = INT_MAX; + + /* If the line is longer than max_width and the column is too close + to max_width, skip part of the line until it is not so close. */ + right_margin = MIN(real_width - s.column, right_margin); + right_margin = max_width - right_margin; + if (real_width >= max_width && s.column > right_margin) + { + line += s.column - right_margin; + s.column = right_margin; + } + + pp_newline (context->printer); + saved_prefix = pp_get_prefix (context->printer); + pp_set_prefix (context->printer, NULL); + pp_character (context->printer, ' '); + while (max_width > 0 && *line != '\0') + { + char c = *line == '\t' ? ' ' : *line; + pp_character (context->printer, c); + max_width--; + line++; + } + pp_newline (context->printer); + /* pp_printf does not implement %*c. */ + buffer = XALLOCAVEC (char, s.column + 3); + snprintf (buffer, s.column + 3, " %*c", s.column, '^'); + pp_string (context->printer, buffer); + pp_set_prefix (context->printer, saved_prefix); +} +
