Skip to content

Commit

Permalink
Pretty errors as default message reporting (#11587)
Browse files Browse the repository at this point in the history
* Switch to pretty errors by default

* Avoid 'Error: : ' in args errors

* Do not eagerly report --cwd errors

* [tests] Lazy migration of misc tests to default pretty errors

* [tests] Don't use pretty errors for display tests

* [tests] More misc tests
  • Loading branch information
kLabz authored Apr 30, 2024
1 parent c325889 commit a88a1a9
Show file tree
Hide file tree
Showing 12 changed files with 32 additions and 13 deletions.
2 changes: 1 addition & 1 deletion src-json/define.json
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,7 @@
{
"name": "MessageReporting",
"define": "message.reporting",
"doc": "Select message reporting mode for compiler output. (default: classic)",
"doc": "Select message reporting mode for compiler output. (default: pretty)",
"params": ["mode: classic | pretty | indent"]
},
{
Expand Down
4 changes: 3 additions & 1 deletion src/compiler/args.ml
Original file line number Diff line number Diff line change
Expand Up @@ -325,11 +325,13 @@ let parse_args com =
List.rev acc
in
let args = loop [] args in
Arg.parse_argv ~current (Array.of_list ("" :: args)) all_args_spec args_callback "";
Arg.parse_argv ~current (Array.of_list ("Haxe" :: args)) all_args_spec args_callback "";
with
| Arg.Help _ ->
raise (Helper.HelpMessage (usage_string all_args usage))
| Arg.Bad msg ->
(* Strip error prefix added by ocaml's arg parser *)
let msg = if ExtLib.String.starts_with msg "Haxe: " then (String.sub msg 6 ((String.length msg) - 6)) else msg in
let first_line = List.nth (Str.split (Str.regexp "\n") msg) 0 in
let new_msg = (Printf.sprintf "%s" first_line) in
let r = Str.regexp "unknown option [`']?\\([-A-Za-z]+\\)[`']?" in
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/compiler.ml
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,8 @@ module HighLevel = struct
loop acc l
| "--cwd" :: dir :: l | "-C" :: dir :: l ->
(* we need to change it immediately since it will affect hxml loading *)
(try Unix.chdir dir with _ -> raise (Arg.Bad ("Invalid directory: " ^ dir)));
(* Exceptions are ignored there to let arg parsing do the error handling in expected order *)
(try Unix.chdir dir with _ -> ());
(* Push the --cwd arg so the arg processor know we did something. *)
loop (dir :: "--cwd" :: acc) l
| "--connect" :: hp :: l ->
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/messageReporting.ml
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ let format_messages com messages =
let absolute_positions = Define.defined com.defines Define.MessageAbsolutePositions in
let ectx = create_error_context absolute_positions in
ectx.max_lines <- get_max_line ectx.max_lines messages;
let message_formatter = get_formatter com Define.MessageReporting "classic" in
let message_formatter = get_formatter com Define.MessageReporting "pretty" in
let lines = List.rev (
List.fold_left (fun lines cm -> match (message_formatter ectx cm) with
| None -> lines
Expand All @@ -372,7 +372,7 @@ let display_messages ctx on_message = begin
compiler_message_string
in

let message_formatter = get_formatter ctx.com Define.MessageReporting "classic" in
let message_formatter = get_formatter ctx.com Define.MessageReporting "pretty" in
let log_formatter = get_formatter ctx.com Define.MessageLogFormat "indent" in

let log_messages = ref (Define.defined ctx.com.defines Define.MessageLogFile) in
Expand Down
2 changes: 1 addition & 1 deletion tests/display/src/BaseDisplayTestContext.hx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class BaseDisplayTestContext {
}

static public function runHaxe(args:Array<String>, ?stdin:String) {
return haxeServer.rawRequest(args, stdin == null ? null : Bytes.ofString(stdin));
return haxeServer.rawRequest(["-D", "message.reporting=classic"].concat(args), stdin == null ? null : Bytes.ofString(stdin));
}

static function normalizePath(p:String):String {
Expand Down
3 changes: 2 additions & 1 deletion tests/misc/flash/projects/Issue9805/compile.hxml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

--next

-D message.reporting=classic
--main Main
--swf-lib bin/lib.swc
--swf bin/test.swf
--swf bin/test.swf
1 change: 1 addition & 0 deletions tests/misc/java/projects/Issue11095/compile-fail.hxml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ NoRudy

--next

-D message.reporting=classic
--main Main
--java-lib no-rudy.jar
--jvm run.jar
9 changes: 8 additions & 1 deletion tests/misc/projects/Issue10871/Compiler/Main.hx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class MacroClass {
public static function start() {
final config = Compiler.getConfiguration();

trace(config.args);
trace(filterArgs(config.args));
trace(config.debug);
trace(config.verbose);
trace(config.foptimize);
Expand All @@ -37,5 +37,12 @@ class MacroClass {
case _: "unused platform";
}
}

static function filterArgs(args:Array<String>):Array<String> {
if (args.length < 2) return args;
// We're currently prepending that to all tests while moving to pretty errors by default
if (args[0] == "-D" && args[1] == "message.reporting=classic") return args.slice(2);
return args;
}
}
#end
11 changes: 9 additions & 2 deletions tests/misc/projects/Issue11087/Main.hx
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
macro function test() {
trace(Sys.args());
trace(filterArgs(Sys.args()));
return macro null;
}

function main() {
test();
trace(Sys.args());
trace(filterArgs(Sys.args()));
}

function filterArgs(args:Array<String>):Array<String> {
if (args.length < 2) return args;
// We're currently prepending that to all tests while moving to pretty errors by default
if (args[0] == "-D" && args[1] == "message.reporting=classic") return args.slice(2);
return args;
}
2 changes: 1 addition & 1 deletion tests/misc/projects/Issue11128/compile2-fail.hxml.stderr
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Error: : --custom-target cannot use reserved name js.
Error: --custom-target cannot use reserved name js.
2 changes: 1 addition & 1 deletion tests/misc/projects/Issue3300/test-cwd-fail.hxml.stderr
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Error: Invalid directory: unexistant
Error: Invalid directory: unexistant.
2 changes: 1 addition & 1 deletion tests/misc/src/Main.hx
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class Main {
var expectFailure = file.endsWith("-fail.hxml");
var expectStdout = if (FileSystem.exists('$file.stdout')) prepareExpectedOutput(File.getContent('$file.stdout')) else null;
var expectStderr = if (FileSystem.exists('$file.stderr')) prepareExpectedOutput(File.getContent('$file.stderr')) else null;
var result = runCommand("haxe", [file], expectFailure, expectStdout, expectStderr);
var result = runCommand("haxe", ["-D", "message.reporting=classic", file], expectFailure, expectStdout, expectStderr);
++count;
if (!result.success) {
failures++;
Expand Down

0 comments on commit a88a1a9

Please sign in to comment.