From patchwork Wed Dec 10 12:20:05 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?b?TWFudWVsIEzDs3Blei1JYsOhw7Fleg==?= X-Patchwork-Id: 419624 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 5141F1400A0 for ; Wed, 10 Dec 2014 23:20:59 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type; q=dns; s=default; b=VOrZ898ae+gbMal YWFCFY+nV8bA6KGoTmppKotyPPVpbQFPjU9d4E4tCDfM2h61Ng6eyg2AUI17T1/k hej7ZsrNeFn3Wc9OwHCH3pLcWZHgex6QFpBNYdH/afRu8mSQlO9+Ggm4bnu17mj5 vC/qf3+bbHb9xNW9CI+qm/OWBP3E= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type; s=default; bh=/BwXEKJaoHe2kLdpEQJdR Ho9J44=; b=GECpiINWSns+poevBR5U5rkTqfBTXXR//IuUkSqJeBb6LmoV9mWw9 18rSn/858hsbieAZDVZ4hpAHSM4cb4d+9eHPbw4J88oWtDpsfejdRmpO79puXLSb fP0eXLG62pZlr1Jzt7ArHaLhaKNh9GHpPTaDu1HMZXDHiNnaU7OfPs= Received: (qmail 11810 invoked by alias); 10 Dec 2014 12:20:51 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 11792 invoked by uid 89); 10 Dec 2014 12:20:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mail-wg0-f54.google.com Received: from mail-wg0-f54.google.com (HELO mail-wg0-f54.google.com) (74.125.82.54) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 10 Dec 2014 12:20:49 +0000 Received: by mail-wg0-f54.google.com with SMTP id l2so3432382wgh.41 for ; Wed, 10 Dec 2014 04:20:46 -0800 (PST) X-Received: by 10.180.84.198 with SMTP id b6mr5912741wiz.41.1418214045646; Wed, 10 Dec 2014 04:20:45 -0800 (PST) MIME-Version: 1.0 Received: by 10.217.141.72 with HTTP; Wed, 10 Dec 2014 04:20:05 -0800 (PST) In-Reply-To: <871to7lq01.fsf@redhat.com> References: <54837FDD.8000107@net-b.de> <871to7lq01.fsf@redhat.com> From: =?ISO-8859-1?Q?Manuel_L=F3pez=2DIb=E1=F1ez?= Date: Wed, 10 Dec 2014 13:20:05 +0100 Message-ID: Subject: Re: [RFC] diagnostics.c: For terminals, restrict messages to terminal width? To: Dodji Seketeli Cc: Tobias Burnus , gcc-patches , gfortran , FX On 10 December 2014 at 12:10, Dodji Seketeli wrote: >> >> Note that -fmessage-length= applies to the error message (wraps) _and_ >> the caret diagnostic (truncates) while the COLUMNS variable _only_ >> applies to the caret diagnostic. (BTW: The documentation currently >> does not mention COLUMNS.) > > I guess we should adjust the documentation to mention COLUMNS. > > Manuel, was there a particular reason to avoid mentioning the COLUMNS > environment variable in the documentation? Not that I remember. Perhaps the documentation should say something like: "The line is truncated to fit into n characters only if the option -fmessage-length=n is given, or if the output is a TTY and the COLUMNS environment variable is set." >> (b) Should ioctl be always used or only for Fortran? > > I'd go for using it in the common diagnostics framework, unless there is > a sound motivated reason. Manuel, do you remember why we didn't query the > TIOCGWINSZ ioctl property to get the terminal size when that capability > was available? I was not aware this possibility even existed. >> Comments? Note that Fortran has this: #ifdef GWINSZ_IN_SYS_IOCTL # include #endif Not sure if this is needed for diagnostics.c or whether it needs some configure magick. I also agree with FX that the function should be named something like get_terminal_width(). In fact, I would argue that the Fortran version should be a wrapper around this one to make the output consistent between the new Fortran diagnostics and the old ones (*_1 variants) while the transition is in progress, even if that means changing the current ordering for Fortran. So far, there does not seem to be any reason to prefer one ordering over the other, but whatever it is chosen, it would be better to be consistent. Thus something like: Index: error.c =================================================================== --- error.c (revision 218410) +++ error.c (working copy) @@ -78,7 +78,7 @@ /* Determine terminal width (for trimming source lines in output). */ static int -get_terminal_width (void) +gfc_get_terminal_width (void) { /* Only limit the width if we're outputting to a terminal. */ #ifdef HAVE_UNISTD_H @@ -85,26 +85,7 @@ if (!isatty (STDERR_FILENO)) return INT_MAX; #endif - - /* Method #1: Use ioctl (not available on all systems). */ -#ifdef TIOCGWINSZ - struct winsize w; - w.ws_col = 0; - if (ioctl (0, TIOCGWINSZ, &w) == 0 && w.ws_col > 0) - return w.ws_col; -#endif - - /* Method #2: Query environment variable $COLUMNS. */ - const char *p = getenv ("COLUMNS"); - if (p) - { - int value = atoi (p); - if (value > 0) - return value; - } - - /* If both fail, use reasonable default. */ - return 80; + return get_terminal_width (); } @@ -113,7 +94,7 @@ void gfc_error_init_1 (void) { - terminal_width = get_terminal_width (); + terminal_width = gfc_get_terminal_width (); errors = 0; warnings = 0; buffer_flag = 0; The other "conflict" is that Fortran's default terminal_width if everything fails is 80, whereas the common diagnostics defaults to INT_MAX. If Fortran devs do not mind this new default (which is already in place for all diagnostics using the new functions), then the wrapper will make the output consistent. If they really want to default to 80, then the Fortran code can still use the wrapper but do "If (terminal_width == INT_MAX) terminal_width = 80" and call diagnostic_set_caret_max_width(, terminal_width) to set the same value to the new and old functions. Finally, there is another use of getenv ("COLUMNS") in opts.c that could use this new function: Index: opts.c =================================================================== --- opts.c (revision 218410) +++ opts.c (working copy) @@ -1231,18 +1231,8 @@ the desired maximum width of the output. */ if (opts->x_help_columns == 0) { - const char *p; - - p = getenv ("COLUMNS"); - if (p != NULL) - { - int value = atoi (p); - - if (value > 0) - opts->x_help_columns = value; - } - - if (opts->x_help_columns == 0) + opts->x_help_columns = get_terminal_width (); + if (opts->x_help_columns == INT_MAX) /* Use a reasonable default. */ opts->x_help_columns = 80; }