diff mbox

[RFC] diagnostics.c: For terminals, restrict messages to terminal width?

Message ID 5488BE75.1020306@net-b.de
State New
Headers show

Commit Message

Tobias Burnus Dec. 10, 2014, 9:43 p.m. UTC
Hi all,

I have now updated the patch, based on all comments and Manuel's patch. 
And bootstrapped it on x86-64-gnu-linux. If there are no more comments, 
I intent to commit it tomorrow. If it gets approved earlier, I will 
commit it earlier ;-)

Some comments from me are below.

FX wrote:
>> So the patch you (Manual) are proposing looks fine to me, with the
>> environment variable taking precedence, *if* that is fine for Fortran,
>> of course.
> That seems fine to me, from the Fortran standpoint. COLUMNS is a bit of a special environment variable, which the shell (when it provides it) keeps synchronized to the terminal width anyway (as is the case for bash and zsh).

Ditto for me, although my feeling is that COLUMNS is only rarely used as 
it is does not seem to get automatically exported by the shells.


Manuel López-Ibáñez wrote:
> 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.

I think that's fine. Most Fortran source code is < 132 characters as 
maximally 132 characters are permitted by the standard (for free-form 
source code);* the number of cases, where it is longer, should be quite 
small and on average is should be even shorter. And terminal/editor 
widths of > 80 are also common, such that viewing a log there, also 
should work without wrapping.

(* gfortran doesn't count comments exceeding that limit.)

Tobias

Comments

Dodji Seketeli Dec. 11, 2014, 8:11 a.m. UTC | #1
Tobias Burnus <burnus@net-b.de> writes:

> 2014-12-06  Tobias Burnus  <burnus@net-b.de>
> 	    Manuel L³pez-Ib¡±ez  <manu@gcc.gnu.org>
>
> gcc/
> 	* diagnostic.c (get_terminal_width): Renamed from getenv_columns,
> 	removed static, and additionally use ioctl to get width.
> 	(diagnostic_set_caret_max_width): Update call.
> 	* diagnostic.h (get_terminal_width): Add prototype.
> 	* opts.c (print_specific_help): Use it for x_help_columns.
> 	* doc/invoke.texi (fdiagnostics-show-caret): Document how the
> 	width is set.
>
> gcc/fortran/
> 	* error.c (gfc_get_terminal_width): Renamed from
> 	get_terminal_width and use same-named common function.
> 	(gfc_error_init_1): Update call.

The diagnostics infrastructure changes are OK for me.  Thanks!

Cheers,
diff mbox

Patch

2014-12-06  Tobias Burnus  <burnus@net-b.de>
	    Manuel López-Ibáñez  <manu@gcc.gnu.org>

gcc/
	* diagnostic.c (get_terminal_width): Renamed from getenv_columns,
	removed static, and additionally use ioctl to get width.
	(diagnostic_set_caret_max_width): Update call.
	* diagnostic.h (get_terminal_width): Add prototype.
	* opts.c (print_specific_help): Use it for x_help_columns.
	* doc/invoke.texi (fdiagnostics-show-caret): Document how the
	width is set.

gcc/fortran/
	* error.c (gfc_get_terminal_width): Renamed from
	get_terminal_width and use same-named common function.
	(gfc_error_init_1): Update call.

diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 541e2fb..41101a2 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -33,6 +33,14 @@  along with GCC; see the file COPYING3.  If not see
 #include "diagnostic.h"
 #include "diagnostic-color.h"
 
+#ifdef HAVE_TERMIOS_H
+# include <termios.h>
+#endif
+
+#ifdef GWINSZ_IN_SYS_IOCTL
+# include <sys/ioctl.h>
+#endif
+
 #include <new>                     // For placement new.
 
 #define pedantic_warning_kind(DC)			\
@@ -81,9 +89,10 @@  file_name_as_prefix (diagnostic_context *context, const char *f)
 
 
 /* Return the value of the getenv("COLUMNS") as an integer. If the
-   value is not set to a positive integer, then return INT_MAX.  */
-static int
-getenv_columns (void)
+   value is not set to a positive integer, use ioctl to get the
+   terminal width. If it fails, return INT_MAX.  */
+int
+get_terminal_width (void)
 {
   const char * s = getenv ("COLUMNS");
   if (s != NULL) {
@@ -91,6 +100,14 @@  getenv_columns (void)
     if (n > 0)
       return n;
   }
+
+#ifdef TIOCGWINSZ
+  struct winsize w;
+  w.ws_col = 0;
+  if (ioctl (0, TIOCGWINSZ, &w) == 0 && w.ws_col > 0)
+    return w.ws_col;
+#endif
+
   return INT_MAX;
 }
 
@@ -101,7 +118,7 @@  diagnostic_set_caret_max_width (diagnostic_context *context, int value)
   /* One minus to account for the leading empty space.  */
   value = value ? value - 1 
     : (isatty (fileno (pp_buffer (context->printer)->stream))
-       ? getenv_columns () - 1: INT_MAX);
+       ? get_terminal_width () - 1: INT_MAX);
   
   if (value <= 0) 
     value = INT_MAX;
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index 807ce91..e699db8 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -298,6 +298,8 @@  void diagnostic_action_after_output (diagnostic_context *, diagnostic_t);
 
 void diagnostic_file_cache_fini (void);
 
+int get_terminal_width (void);
+
 /* Expand the location of this diagnostic. Use this function for consistency. */
 
 static inline expanded_location
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index d2f3c79..40eb8b6 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -3187,7 +3187,10 @@  option is known to the diagnostic machinery).  Specifying the
 @opindex fdiagnostics-show-caret
 By default, each diagnostic emitted includes the original source line
 and a caret '^' indicating the column.  This option suppresses this
-information.
+information.  The source line is truncated to @var{n} characters, if
+the @option{-fmessage-length=n} is given.  When the output is done
+to the terminal, the width is limited to the width given by the
+@env{COLUMNS} environment variable or, if not set, to the terminal width.
 
 @end table
 
diff --git a/gcc/fortran/error.c b/gcc/fortran/error.c
index 541a799..75407dc 100644
--- a/gcc/fortran/error.c
+++ b/gcc/fortran/error.c
@@ -30,14 +30,6 @@  along with GCC; see the file COPYING3.  If not see
 #include "flags.h"
 #include "gfortran.h"
 
-#ifdef HAVE_TERMIOS_H
-# include <termios.h>
-#endif
-
-#ifdef GWINSZ_IN_SYS_IOCTL
-# include <sys/ioctl.h>
-#endif
-
 #include "diagnostic.h"
 #include "diagnostic-color.h"
 #include "tree-diagnostic.h" /* tree_diagnostics_defaults */
@@ -91,33 +83,9 @@  gfc_pop_suppress_errors (void)
 /* 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
-  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 isatty (STDERR_FILENO) ? get_terminal_width () : INT_MAX;
 }
 
 
@@ -126,7 +94,7 @@  get_terminal_width (void)
 void
 gfc_error_init_1 (void)
 {
-  terminal_width = get_terminal_width ();
+  terminal_width = gfc_get_terminal_width ();
   errors = 0;
   warnings = 0;
   gfc_buffer_error (false);
diff --git a/gcc/opts.c b/gcc/opts.c
index 1b4f97e..34a42a5 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1231,18 +1231,8 @@  print_specific_help (unsigned int include_flags,
      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;
     }