[m-rev.] for review: Conditionally enable color diagnostics by default.
Peter Wang
novalazy at gmail.com
Tue Jun 25 11:22:40 AEST 2024
Questions:
Do we need config_default_color_diagnostics?
mmake always redirects output to .err files; by default the .err files
will contain uncolored output. If you want colored output in those files,
you will need to explicitly ask for it. I think this is fine, but maybe
there are differing opinions.
----
If color diagnostics are not explicitly enabled or disabled
(by any means), then enable color diagnostics, but only if
standard error is a terminal. The same setting will affect
diagnostic messages placed into .err files by mmc --make.
compiler/handle_options.m:
As above.
compiler/make.build.m:
Add --color-diagnostics or --no-color-diagnostics to the list of
options in setup_for_build_with_module_options. This is to prevent
further invocations of the Mercury compiler from checking whether
standard error is a terminal again.
compiler/make.get_module_dep_info.m:
compiler/make.module_target.m:
compiler/make.program_target.m:
compiler/mercury_compile_main.m:
Conform to changes.
compiler/options.m:
List --color-diagnostics in help text, as passing that option now
differs from the default behaviour.
doc/user_guide.texi:
Delete redundant documentation for the --color-scheme option.
List --color-diagnostics option.
Update the description of how the compiler determines whether to
enable color diagnostics. Simplify the text.
diff --git a/compiler/handle_options.m b/compiler/handle_options.m
index 05e6baec5..0df42c288 100644
--- a/compiler/handle_options.m
+++ b/compiler/handle_options.m
@@ -3176,15 +3176,6 @@ handle_compare_specialization(!Globals) :-
:- pred handle_colors(globals::in, globals::out, io::di, io::uo) is det.
handle_colors(!Globals, !IO) :-
- % NOTE This predicate does not yet handle the issue of whether
- % the output is going to a tty or not. If and when it does want to do so,
- % it will first have to *figure out* where the output is going.
- % Given that
- %
- % - mmc --make first puts all diagnostics in .err files, but then
- % - copies the first N lines of those diagnostics to stderr,
- %
- % do we want to enable colors only in the first N lines of diagnostics?
globals.lookup_bool_option(!.Globals,
color_diagnostics_is_set, EnableIsSet),
globals.lookup_bool_option(!.Globals,
@@ -3197,11 +3188,48 @@ handle_colors(!Globals, !IO) :-
UseColor = EnableValue
;
EnableIsSet = no,
- % If the user dod not set the enable option, use the default.
- UseColor = ConfigDefault
+ % If the user did not explicitly enable or disable color, then we
+ % enable color based on whether standard error is associated with a
+ % terminal device.
+ %
+ % NOTE Since mmc --make writes diagnostics into both .err files *and*
+ % to stderr, there is an argument to be made that the two outputs
+ % should be independent: the output going to a terminal could contain
+ % color escape sequences, but the output going into .err files might
+ % not. That would be a non-trivial change as mmc --make works by
+ % writing diagnostics into a .err file, then copying the first N lines
+ % to stderr. (Writing to a file first also prevents interleaved output
+ % from parallel tasks.)
+ %
+ % In the absence of support for producing independent outputs, and an
+ % option to control that behaviour, we will simply produce the same
+ % output to stderr and .err files.
+ check_stderr_is_terminal(IsTerminal, !IO),
+ (
+ IsTerminal = yes,
+ % If the user did not set the enable option, use the default.
+ % XXX can we just make this yes?
+ UseColor = ConfigDefault
+ ;
+ IsTerminal = no,
+ UseColor = no
+ )
),
globals.set_option(use_color_diagnostics, bool(UseColor), !Globals).
+:- pred check_stderr_is_terminal(bool::out, io::di, io::uo) is det.
+
+:- pragma foreign_proc("C",
+ check_stderr_is_terminal(IsTerminal::out, _IO0::di, _IO::uo),
+ [will_not_call_mercury, promise_pure, thread_safe, may_not_duplicate],
+"
+#ifdef MR_HAVE_ISATTY
+ IsTerminal = (isatty(STDERR_FILENO) == 1) ? MR_YES : MR_NO;
+#else
+ IsTerminal = MR_NO;
+#endif
+").
+
%---------------------%
% Options updated:
diff --git a/compiler/make.build.m b/compiler/make.build.m
index a570d92b3..96d652c10 100644
--- a/compiler/make.build.m
+++ b/compiler/make.build.m
@@ -26,6 +26,7 @@
:- import_module parse_tree.
:- import_module parse_tree.error_spec.
+:- import_module bool.
:- import_module getopt.
:- import_module io.
:- import_module list.
@@ -46,16 +47,16 @@
% for the build.
% setup_for_build_with_module_options(ProgressStream, DefaultOptionTable,
- % InvokedByMmcMake, ModuleName, DetectedGradeFlags,
+ % InvokedByMmcMake, UseColors, ModuleName, DetectedGradeFlags,
% OptionVariables, EnvVarArgs, OptionArgs, ExtraOptions, MayBuild,
% !Info, !IO):
%
% Set up for building some compiler-generated file for ModuleName,
% Return, in MayBuild, the full argument list for that compiler invocation,
% containing module-specific options from OptionVariables and OptionArgs,
- % and including ExtraOptions, adding `--use-subdirs' and
- % `--invoked-by-mmc-make' to the option list. (The latter presumably
- % dependent on the value of the second arg).
+ % and including ExtraOptions, adding `--use-subdirs',
+ % `--invoked-by-mmc-make' and `--[no-]color-diagnostics' to the option
+ % list.
%
% Return next to it a version of the globals structure that results
% from this full argument list.
@@ -67,7 +68,7 @@
% or possibly just a maybe(op_mode). not list(string),
%
:- pred setup_for_build_with_module_options(io.text_output_stream::in,
- option_table(option)::in, maybe_invoked_by_mmc_make::in,
+ option_table(option)::in, maybe_invoked_by_mmc_make::in, bool::in,
module_name::in, list(string)::in, options_variables::in,
list(string)::in, list(string)::in, list(string)::in,
may_build::out, io::di, io::uo) is det.
@@ -215,7 +216,6 @@
:- import_module parse_tree.file_names.
:- import_module parse_tree.maybe_error.
-:- import_module bool.
:- import_module char.
:- import_module int.
:- import_module io.file.
@@ -227,8 +227,9 @@
%---------------------------------------------------------------------------%
setup_for_build_with_module_options(ProgressStream, DefaultOptionTable,
- InvokedByMmcMake, ModuleName, DetectedGradeFlags, OptionVariables,
- EnvVarArgs, OptionArgs, ExtraOptions, MayBuild, !IO) :-
+ InvokedByMmcMake, UseColors, ModuleName, DetectedGradeFlags,
+ OptionVariables, EnvVarArgs, OptionArgs, ExtraOptions, MayBuild,
+ !IO) :-
lookup_mmc_module_options(OptionVariables, ModuleName,
MaybeModuleOptionArgs),
(
@@ -243,14 +244,24 @@ setup_for_build_with_module_options(ProgressStream, DefaultOptionTable,
% assumes the interface files were built with `--use-subdirs'.
(
InvokedByMmcMake = invoked_by_mmc_make,
- UseSubdirs = ["--use-subdirs"],
- InvokedByMake = ["--invoked-by-mmc-make"]
+ InvokedByMake = ["--invoked-by-mmc-make"],
+ UseSubdirs = ["--use-subdirs"]
;
InvokedByMmcMake = not_invoked_by_mmc_make,
- UseSubdirs = [],
- InvokedByMake = []
+ InvokedByMake = [],
+ UseSubdirs = []
),
- AllOptionArgs = InvokedByMake ++ DetectedGradeFlags ++
+ % Adding --color-diagnostics or --no-color-diagnostics prevents
+ % handle_given_options or a Mercury compiler sub-process from checking
+ % if standard error is a terminal again.
+ (
+ UseColors = yes,
+ ColorOptions = ["--color-diagnostics"]
+ ;
+ UseColors = no,
+ ColorOptions = ["--no-color-diagnostics"]
+ ),
+ AllOptionArgs = InvokedByMake ++ ColorOptions ++ DetectedGradeFlags ++
ModuleOptionArgs ++ EnvVarArgs ++ OptionArgs ++
ExtraOptions ++ UseSubdirs,
handle_given_options(ProgressStream, DefaultOptionTable, AllOptionArgs,
diff --git a/compiler/make.get_module_dep_info.m b/compiler/make.get_module_dep_info.m
index 23db18fb1..ce31137f9 100644
--- a/compiler/make.get_module_dep_info.m
+++ b/compiler/make.get_module_dep_info.m
@@ -633,13 +633,14 @@ write_module_dep_files_for_source_file(Globals, ProgressStream,
setup_checking_for_interrupt(CookieMSI, !IO),
globals.get_default_options(Globals, DefaultOptionTable),
+ globals.lookup_bool_option(Globals, use_color_diagnostics, UseColors),
DetectedGradeFlags = make_info_get_detected_grade_flags(!.Info),
OptionVariables = make_info_get_options_variables(!.Info),
EnvVarArgs = make_info_get_env_var_args(!.Info),
OptionArgs = make_info_get_option_args(!.Info),
ExtraOptions = ["--make-short-interface"],
setup_for_build_with_module_options(ProgressStream, DefaultOptionTable,
- invoked_by_mmc_make, ModuleName, DetectedGradeFlags,
+ invoked_by_mmc_make, UseColors, ModuleName, DetectedGradeFlags,
OptionVariables, EnvVarArgs, OptionArgs, ExtraOptions,
MayBuild, !IO),
(
diff --git a/compiler/make.module_target.m b/compiler/make.module_target.m
index 75a1b3bb6..8ffe9f1da 100644
--- a/compiler/make.module_target.m
+++ b/compiler/make.module_target.m
@@ -341,12 +341,13 @@ build_target(ProgressStream, Globals, CompilationTask,
MakeLhsFiles),
setup_checking_for_interrupt(Cookie, !IO),
get_default_options(Globals, DefaultOptionTable),
+ globals.lookup_bool_option(Globals, use_color_diagnostics, UseColors),
DetectedGradeFlags = make_info_get_detected_grade_flags(!.Info),
OptionVariables = make_info_get_options_variables(!.Info),
EnvVarArgs = make_info_get_env_var_args(!.Info),
OptionArgs = make_info_get_option_args(!.Info),
setup_for_build_with_module_options(ProgressStream, DefaultOptionTable,
- invoked_by_mmc_make, ModuleName,
+ invoked_by_mmc_make, UseColors, ModuleName,
DetectedGradeFlags, OptionVariables, EnvVarArgs, OptionArgs,
ExtraAndTaskOptions, MayBuild, !IO),
(
diff --git a/compiler/make.program_target.m b/compiler/make.program_target.m
index 708b37f65..6f3b37d1c 100644
--- a/compiler/make.program_target.m
+++ b/compiler/make.program_target.m
@@ -186,14 +186,15 @@ make_linked_target_1(Globals, LinkedTargetFile, ExtraOptions,
(
IntermodAnalysisSucceeded = succeeded,
get_default_options(Globals, DefaultOptionTable),
+ globals.lookup_bool_option(Globals, use_color_diagnostics, UseColors),
DetectedGradeFlags = make_info_get_detected_grade_flags(!.Info),
OptionVariables = make_info_get_options_variables(!.Info),
EnvVarArgs = make_info_get_env_var_args(!.Info),
OptionArgs = make_info_get_option_args(!.Info),
setup_for_build_with_module_options(ProgressStream, DefaultOptionTable,
- invoked_by_mmc_make, MainModuleName, DetectedGradeFlags,
- OptionVariables, EnvVarArgs, OptionArgs, ExtraOptions,
- MayBuild, !IO),
+ invoked_by_mmc_make, UseColors, MainModuleName, DetectedGradeFlags,
+ OptionVariables, EnvVarArgs, OptionArgs, ExtraOptions, MayBuild,
+ !IO),
(
MayBuild = may_build(_AllOptionArgs, BuildGlobals),
make_linked_target_2(ProgressStream, BuildGlobals,
@@ -951,14 +952,16 @@ do_not_reinsert_java_class_timestamps(FileName, MaybeTimestamp, !Timestamps) :-
make_misc_target(ProgressStream, Globals, MainModuleName - TargetType,
Succeeded, !Info, !Specs, !IO) :-
get_default_options(Globals, DefaultOptionTable),
+ globals.lookup_bool_option(Globals, use_color_diagnostics, UseColors),
DetectedGradeFlags = make_info_get_detected_grade_flags(!.Info),
OptionVariables = make_info_get_options_variables(!.Info),
EnvVarArgs = make_info_get_env_var_args(!.Info),
OptionArgs = make_info_get_option_args(!.Info),
ExtraOptions = [],
setup_for_build_with_module_options(ProgressStream, DefaultOptionTable,
- invoked_by_mmc_make, MainModuleName, DetectedGradeFlags,
- OptionVariables, EnvVarArgs, OptionArgs, ExtraOptions, MayBuild, !IO),
+ invoked_by_mmc_make, UseColors, MainModuleName, DetectedGradeFlags,
+ OptionVariables, EnvVarArgs, OptionArgs, ExtraOptions,
+ MayBuild, !IO),
(
MayBuild = may_build(_AllOptionArgs, BuildGlobals),
make_misc_target_builder(ProgressStream, BuildGlobals, MainModuleName,
diff --git a/compiler/mercury_compile_main.m b/compiler/mercury_compile_main.m
index 8dedb9c66..5cb60aa75 100644
--- a/compiler/mercury_compile_main.m
+++ b/compiler/mercury_compile_main.m
@@ -1012,10 +1012,12 @@ generate_executable(ProgressStream, ErrorStream, Globals, InvokedByMmcMake,
;
InvokedByMmcMake = op_mode_not_invoked_by_mmc_make,
get_default_options(Globals, DefaultOptionTable),
+ globals.lookup_bool_option(Globals, use_color_diagnostics,
+ UseColors),
setup_for_build_with_module_options(ProgressStream,
- DefaultOptionTable, not_invoked_by_mmc_make, MainModuleName,
- DetectedGradeFlags, OptionVariables, EnvVarArgs, OptionArgs,
- [], MayBuild, !IO),
+ DefaultOptionTable, not_invoked_by_mmc_make, UseColors,
+ MainModuleName, DetectedGradeFlags, OptionVariables,
+ EnvVarArgs, OptionArgs, [], MayBuild, !IO),
(
MayBuild = may_not_build(Specs),
Succeeded = did_not_succeed
@@ -1164,11 +1166,12 @@ setup_and_process_compiler_arg(ProgressStream, ErrorStream, Globals,
OptionVariables, EnvVarArgs, OptionArgs, Arg,
ModulesToLink, ExtraObjFiles, !HaveParseTreeMaps, !Specs, !IO) :-
get_default_options(Globals, DefaultOptionTable),
+ globals.lookup_bool_option(Globals, use_color_diagnostics, UseColors),
FileOrModule = string_to_file_or_module(Arg),
ModuleName = file_or_module_to_module_name(FileOrModule),
ExtraOptions = [],
setup_for_build_with_module_options(ProgressStream, DefaultOptionTable,
- not_invoked_by_mmc_make, ModuleName, DetectedGradeFlags,
+ not_invoked_by_mmc_make, UseColors, ModuleName, DetectedGradeFlags,
OptionVariables, EnvVarArgs, OptionArgs, ExtraOptions, MayBuild, !IO),
(
MayBuild = may_not_build(SetupSpecs),
diff --git a/compiler/options.m b/compiler/options.m
index 6eaeb124b..9b7ab0480 100644
--- a/compiler/options.m
+++ b/compiler/options.m
@@ -2376,8 +2376,8 @@ long_table("config-default-color-diagnostics",
config_default_color_diagnostics).
long_table("config-default-colour-diagnostics",
config_default_color_diagnostics).
-long_table("color-diagnostics", color_diagnostics).
-long_table("colour-diagnostics", color_diagnostics).
+long_table("color-diagnostics", color_diagnostics).
+long_table("colour-diagnostics", color_diagnostics).
% use_color_diagnostics is an internal-use-only option.
long_table("color-scheme", color_scheme).
long_table("colour-scheme", color_scheme).
@@ -4735,10 +4735,10 @@ options_help_verbosity(Stream, !IO) :-
"\tsection named \"Color schemes\" in the Mercury user's Guide",
"\tfor the details.",
- "--no-color-diagnostics",
- "\tDisable the use of colors in diagnostic messages. Please see",
- "\tthe section named \"Enabling the use of color\" section in the",
- "\tMercury Users's Guide for details.",
+ "--color-diagnostics, --no-color-diagnostics",
+ "\tEnable or disable the use of colors in diagnostic messages.",
+ "\tPlease see the section named \"Enabling the use of color\"",
+ "\tin the Mercury Users's Guide for details.",
% These work only if the compiler was compiled with
% "--trace-flag type_checkpoint".
diff --git a/doc/user_guide.texi b/doc/user_guide.texi
index 028739e99..ee5908f49 100644
--- a/doc/user_guide.texi
+++ b/doc/user_guide.texi
@@ -7173,21 +7173,15 @@ For information about how the compiler uses colors in diagnostic messages,
and about the syntax of color scheme specifications,
please see the @ref{Color schemes} section for the details.
- at item --color-scheme @var{ColorScheme}
- at itemx --colour-scheme @var{ColorScheme}
- at findex --color-scheme
- at findex --colour-scheme
-Tell the compiler to use the colors specified by @var{ColorScheme}
-when printing diagnostics.
-Please see @ref{Enabling the use of color} for the details.
-
+ at item --color-diagnostics
+ at itemx --colour-diagnostics
@item --no-color-diagnostics
@itemx --no-colour-diagnostics
@findex --color-diagnostics
@findex --colour-diagnostics
@findex --no-color-diagnostics
@findex --no-colour-diagnostics
-Disable the use of colors in diagnostic messages.
+Explicitly enable or disable the use of colors in diagnostic messages.
Please see @ref{Enabling the use of color} for the details.
@c @sp 1
@@ -11852,45 +11846,40 @@ while the second is not;
it is in fact a defacto standard for disabling color.
@samp{https://no-color.org}.
-The rules the compiler uses to decide
-whether the use of color in diagnostics is enabled
-are as follows.
+The compiler decides
+whether to enable the use of color in diagnostics
+by the following rules:
@enumerate
@item
-If the command line contains a @samp{--color-diagnostics} option,
-in either its positive or negative form,
+If the command line contains a @samp{--color-diagnostics}
+or @samp{--no-color-diagnostics} option,
then the compiler will obey it.
@item
-If the command line contains no @samp{--color-diagnostics} option,
-but the environment variable @samp{MERCURY_ENABLE_COLOR} exists
-and contains either @samp{never} or @samp{always},
+Otherwise,
+if the environment variable @samp{MERCURY_ENABLE_COLOR} exists
+and contains a valid value,
then the compiler will obey it.
-The compiler also allows @samp{0} as a synonym for @samp{never},
-and @samp{1} as a synonym for @samp{always}.
+Valid values are @samp{always} or @samp{1} to enable color,
+and @samp{never} or @samp{0} to disable color.
@item
-If the command line contains no @samp{--color-diagnostics} option,
-and the environment variable @samp{MERCURY_ENABLE_COLOR}
-either does not exist, or contains a string other than the four listed above,
-but the environment variable @samp{NO_COLOR} exists
+Otherwise,
+if the environment variable @samp{NO_COLOR} exists
and contains a nonempty string,
then the compiler will obey it by disabling the use of color.
@item
-If the command line contains no @samp{--color-diagnostics} option
-and neither environment variable has a value
-that directs the compiler to obey it,
-but the files named in any @samp{--options-file} compiler options,
+Otherwise,
+if the files named in any @samp{--options-file} compiler options,
(or, in the absence of such options, the @samp{Mercury.options} file),
-include a @samp{--color-diagnostics} option,
-in either its positive or negative form,
+include a @samp{--color-diagnostics} or @samp{--no-color-diagnostics} option
among the module-specific @samp{MCFLAGS} for the current module,
then the compiler will obey it.
@item
-If the preconditions of all four of the above rules are false,
-i.e. none of them decide the outcome,
-then the compiler will fall back to its default.
-The usual default decision is to enable the use of color,
-though this can be overridden for a given installation of Mercury.
+Otherwise,
+the compiler will automatically enable color in diagnostics
+if the standard error stream is associated with a terminal device,
+ at c XXX can we get rid of this complication?
+unless this has been overridden in the Mercury installation.
@end enumerate
@c ----------------------------------------------------------------------------
--
2.44.0
More information about the reviews
mailing list