On 2/1/20 9:55 AM, Jakub Jelinek wrote:
> On Sat, Feb 01, 2020 at 07:30:15AM +0000, Bernd Edlinger wrote:
>> @@ -239,20 +243,86 @@ colorize_init (diagnostic_color_rule_t rule)
>>      }
>>  }
>>  
>> +/* Return URL_FORMAT_XXX which tells how we should emit urls
>> +   when in always mode.
>> +   We use GCC_URLS and if that is not defined TERM_URLS.
>> +   If neither is defined the feature is enabled by default.  */
>> +
>> +static diagnostic_url_format
>> +parse_gcc_urls()
> 
> Formatting, missing space before (.
> 

Gotcha :) this is openssl style sneaking in...

fixed.

>> diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
>> index 6ffafac..59d7ecd 100644
>> --- a/gcc/doc/install.texi
>> +++ b/gcc/doc/install.texi
>> @@ -2097,9 +2097,18 @@ option (if not used explicitly on the command line).  
>> @var{choice}
>>  can be one of @samp{never}, @samp{auto}, @samp{always}, and 
>> @samp{auto-if-env}
>>  where @samp{auto} is the default.  @samp{auto-if-env} means that
>>  @option{-fdiagnostics-color=auto} will be the default if @code{GCC_COLORS}
>> -is present and non-empty in the environment, and
>> +is present and non-empty in the environment at runtime, and
>>  @option{-fdiagnostics-color=never} otherwise.
> 
> I'm not sure "at runtime" is the right term here, usually we talk about
> runtime when running the program produced by gcc, not about compile time.
> So perhaps try to be even more verbose and say "in the environment of the
> compiler" or so?
> 

way better, thanks.

>> +The default depends on how the compiler has been configured.
>> +it can be any of the above @var{WHEN} options.
> 
> After full stop next sentence should start with capital letter.
> 

fixed.

>> +
>> +GCC can also be configured (via the
>> +@option{--with-diagnostics-urls=auto-if-env} configure-time option)
>> +so that the default is affected by environment variables.
>> +Under such a configuration, GCC defaults to using @samp{auto}
>> +if either @env{GCC_URLS} or @env{TERM_URLS} environment variables are
>> +present in the environment, or @samp{never} if neither are.
>> +
>> +But even with @option{-fdiagnostics-urls=always} the behavior will be
>> +dependent on those environmen variables:

ah, typo...

> 
> I know I start sentences with But all the time, but some style guides are
> against that, not sure if we want to do that in the documentation.
> 

okay I can start the sentence now with "However, even with..."

In the sentence above I should probably also say are "present _and non-empty_ 
in the
environment _of the compiler_".

So here is just an incremental update with the improvements so far:

diff --git a/gcc/diagnostic-color.c b/gcc/diagnostic-color.c
index f406139..73ca771 100644
--- a/gcc/diagnostic-color.c
+++ b/gcc/diagnostic-color.c
@@ -249,7 +249,7 @@ colorize_init (diagnostic_color_rule_t rule)
    If neither is defined the feature is enabled by default.  */
 
 static diagnostic_url_format
-parse_gcc_urls()
+parse_gcc_urls ()
 {
   const char *p;
 
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 59d7ecd..1d3fec5 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -2097,7 +2097,7 @@ option (if not used explicitly on the command line).  
@var{choice}
 can be one of @samp{never}, @samp{auto}, @samp{always}, and @samp{auto-if-env}
 where @samp{auto} is the default.  @samp{auto-if-env} means that
 @option{-fdiagnostics-color=auto} will be the default if @code{GCC_COLORS}
-is present and non-empty in the environment at runtime, and
+is present and non-empty in the environment of the compiler, and
 @option{-fdiagnostics-color=never} otherwise.
 
 @item --with-diagnostics-urls=@var{choice}
@@ -2106,8 +2106,8 @@ option (if not used explicitly on the command line).  
@var{choice}
 can be one of @samp{never}, @samp{auto}, @samp{always}, and @samp{auto-if-env}
 where @samp{auto} is the default.  @samp{auto-if-env} means that
 @option{-fdiagnostics-urls=auto} will be the default if @env{GCC_URLS}
-or @env{TERM_URLS} is present and non-empty in the environment at runtime, and
-@option{-fdiagnostics-urls=never} otherwise.
+or @env{TERM_URLS} is present and non-empty in the environment of the
+compiler, and @option{-fdiagnostics-urls=never} otherwise.
 
 @item --enable-lto
 @itemx --disable-lto
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 6df0efd..59306bc 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -4044,18 +4044,19 @@ option.
 is a terminal, and @env{TERM} is not set to "dumb".
 
 The default depends on how the compiler has been configured.
-it can be any of the above @var{WHEN} options.
+It can be any of the above @var{WHEN} options.
 
 GCC can also be configured (via the
 @option{--with-diagnostics-urls=auto-if-env} configure-time option)
 so that the default is affected by environment variables.
 Under such a configuration, GCC defaults to using @samp{auto}
 if either @env{GCC_URLS} or @env{TERM_URLS} environment variables are
-present in the environment, or @samp{never} if neither are.
+present and non-empty in the environment of the compiler, or @samp{never}
+if neither are.
 
-But even with @option{-fdiagnostics-urls=always} the behavior will be
-dependent on those environmen variables:
-If @env{GCC_URLS} set to empty or "no" do not embed URLs in diagnostics.
+However, even with @option{-fdiagnostics-urls=always} the behavior will be
+dependent on those environment variables:
+If @env{GCC_URLS} set to empty or "no", do not embed URLs in diagnostics.
 If set to "st", URLs use ST escape sequences.
 If set to "bel", the default, URLs use BEL escape sequences.
 Any other non-empty value enables the feature.


Thanks
Bernd.

Reply via email to