Skip to content

Commit 5c7f608

Browse files
committed
Reduce color numbers, remove semantic tags, simplify setup
1 parent 1cbe538 commit 5c7f608

File tree

6 files changed

+35
-44
lines changed

6 files changed

+35
-44
lines changed

jscomp/ext/ext_color.ml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,11 @@ let style_of_tag s = match s with
8181
| "error" -> [Bold; FG Red]
8282
| "warning" -> [Bold; FG Magenta]
8383
| "info" -> [Bold; FG Yellow]
84+
| "dim" -> [Dim]
85+
| "filename" -> [FG Cyan]
8486
| _ -> []
8587

86-
let ansi_of_tag ?(style_of_tag=style_of_tag) s =
88+
let ansi_of_tag s =
8789
let l = style_of_tag s in
8890
let s = String.concat ";" (List.map code_of_style l) in
8991
"\x1b[" ^ s ^ "m"

jscomp/ext/ext_color.mli

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,6 @@ type style
3939
| Dim
4040

4141
(** Input is the tag for example `@{<warning>@}` return escape code *)
42-
val ansi_of_tag : ?style_of_tag:(string -> style list) -> string -> string
42+
val ansi_of_tag : string -> string
4343

4444
val reset_lit : string

jscomp/super_errors/super_location.ml

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -45,25 +45,30 @@ let print_loc ppf loc =
4545
end
4646
;;
4747

48-
let print intro ppf loc =
48+
let print ~is_warning intro ppf loc =
4949
setup_colors ();
5050
(* TODO: handle locations such as _none_ and "" *)
5151
if loc.loc_start.pos_fname = "//toplevel//"
5252
&& highlight_locations ppf [loc] then ()
5353
else
54-
fprintf ppf "@[@{<intro>%s@}@]@," intro;
54+
if is_warning then
55+
fprintf ppf "@[@{<info>%s@}@]@," intro
56+
else begin
57+
fprintf ppf "@[@{<error>%s@}@]@," intro
58+
end;
5559
fprintf ppf "@[%a@]@,@," print_loc loc;
5660
let (file, start_line, start_char) = Location.get_pos_info loc.loc_start in
5761
let (_, end_line, end_char) = Location.get_pos_info loc.loc_end in
5862
(* things to special-case: startchar & endchar2 both -1 *)
5963
if start_char == -1 || end_char == -1 then
6064
(* happens sometimes. Syntax error for example *)
61-
fprintf ppf "This is likely a syntax error. The more relevant message should be just above!@ If it's not, please file an issue here:@ github.com/facebook/reason/issues@,"
65+
fprintf ppf "Is there an error before this one? If so, it's likely a syntax error. The more relevant message should be just above!@ If it's not, please file an issue here:@ github.com/facebook/reason/issues@,"
6266
else begin
6367
try
6468
let lines = file_lines file in
6569
fprintf ppf "%a"
6670
(Super_misc.print_file
71+
~is_warning
6772
~lines
6873
~range:(
6974
(start_line, start_char + 1), (* make everything 1-index based. See justifications in Super_mic.print_file *)
@@ -79,14 +84,6 @@ let print intro ppf loc =
7984
(* taken from https://github.com/ocaml/ocaml/blob/4.02/parsing/location.ml#L337 *)
8085
(* This is the error report entry point. We'll replace the default reporter with this one. *)
8186
let rec super_error_reporter ppf ({Location.loc; msg; sub; if_highlight} as err) =
82-
Super_misc.colorize_tagged_string ppf (function
83-
| "intro" -> [Bold; FG Red]
84-
| "filename" -> [FG Cyan]
85-
| "affected_line_number" -> [Bold; FG Red]
86-
| "affected_line_content" -> [FG Red]
87-
| "separator" -> [Dim]
88-
| _ -> []
89-
);
9087
let highlighted =
9188
if if_highlight <> "" then
9289
let rec collect_locs locs {Location.loc; sub; if_highlight; _} =
@@ -100,8 +97,9 @@ let rec super_error_reporter ppf ({Location.loc; msg; sub; if_highlight} as err)
10097
if highlighted then
10198
Format.pp_print_string ppf if_highlight
10299
else begin
100+
Super_misc.setup_colors ppf;
103101
(* open a vertical box. Everything in our message is indented 2 spaces *)
104-
Format.fprintf ppf "@[<v 2>@,%a@,%s@,@]" (print "We've found a bug for you!") loc msg;
102+
Format.fprintf ppf "@[<v 2>@,%a@,%s@,@]" (print ~is_warning:false "We've found a bug for you!") loc msg;
105103
List.iter (Format.fprintf ppf "@,@[%a@]" super_error_reporter) sub;
106104
(* no need to flush here; location's report_exception (which uses this ultimately) flushes *)
107105
end
@@ -110,18 +108,11 @@ let rec super_error_reporter ppf ({Location.loc; msg; sub; if_highlight} as err)
110108
(* This is the warning report entry point. We'll replace the default printer with this one *)
111109
let super_warning_printer loc ppf w =
112110
if Warnings.is_active w then begin
113-
Super_misc.colorize_tagged_string ppf (function
114-
| "intro" -> [Bold; FG Yellow]
115-
| "filename" -> [FG Cyan]
116-
| "affected_line_number" -> [Bold; FG Yellow]
117-
| "affected_line_content" -> [FG Yellow]
118-
| "separator" -> [Dim]
119-
| _ -> []
120-
);
111+
Super_misc.setup_colors ppf;
121112
Misc.Color.setup !Clflags.color;
122113
(* open a vertical box. Everything in our message is indented 2 spaces *)
123114
Format.fprintf ppf "@[<v 2>@,%a@,%a@,@]"
124-
(print ("Warning number " ^ (Super_warnings.number w |> string_of_int)))
115+
(print ~is_warning:true ("Warning number " ^ (Super_warnings.number w |> string_of_int)))
125116
loc
126117
Super_warnings.print
127118
w

jscomp/super_errors/super_misc.ml

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ let leading_space_count str =
3232
starting from the first erroring character?) *)
3333
(* Range coordinates all 1-indexed, like for editors. Otherwise this code
3434
would have way too many off-by-one errors *)
35-
let print_file ~range:((start_line, start_char), (end_line, end_char)) ~lines ppf () =
35+
let print_file ~is_warning ~range:((start_line, start_char), (end_line, end_char)) ~lines ppf () =
3636
(* show 2 lines before & after the erroring lines *)
3737
let first_shown_line = max 1 (start_line - 2) in
3838
let last_shown_line = min (Array.length lines) (end_line + 2) in
@@ -62,6 +62,11 @@ let print_file ~range:((start_line, start_char), (end_line, end_char)) ~lines pp
6262
(* btw, these are unicode chars. They're not of length 1. Careful; we need to
6363
explicitly tell Format to treat them as length 1 below *)
6464
let separator = if columns_to_cut = 0 then "" else "" in
65+
(* coloring *)
66+
let (highlighted_line_number, highlighted_content): (string -> string -> unit, Format.formatter, unit) format * (unit, Format.formatter, unit) format =
67+
if is_warning then ("@{<info>%s@}@{<dim> @<1>%s @}", "@{<info>")
68+
else ("@{<error>%s@}@{<dim> @<1>%s @}", "@{<error>")
69+
in
6570

6671
fprintf ppf "@[<v 0>";
6772
(* inclusive *)
@@ -77,10 +82,10 @@ let print_file ~range:((start_line, start_char), (end_line, end_char)) ~lines pp
7782
(* this is where you insrt the vertical separator. Mark them as legnth 1 as explained above *)
7883
if i < start_line || i > end_line then begin
7984
(* normal, non-highlighted line *)
80-
fprintf ppf "%s@{<separator> @<1>%s @}" padded_line_number separator
85+
fprintf ppf "%s@{<dim> @<1>%s @}" padded_line_number separator
8186
end else begin
82-
(* highlighted row *)
83-
fprintf ppf "@{<affected_line_number>%s@}@{<separator> @<1>%s @}" padded_line_number separator
87+
(* highlighted *)
88+
fprintf ppf highlighted_line_number padded_line_number separator
8489
end;
8590

8691
fprintf ppf "@]"; (* h *)
@@ -89,7 +94,7 @@ let print_file ~range:((start_line, start_char), (end_line, end_char)) ~lines pp
8994

9095
let current_line_strictly_between_start_and_end_line = i > start_line && i < end_line in
9196

92-
if current_line_strictly_between_start_and_end_line then fprintf ppf "@{<affected_line_content>";
97+
if current_line_strictly_between_start_and_end_line then fprintf ppf highlighted_content;
9398

9499
let current_line_cut_length = String.length current_line_cut in
95100
(* inclusive. To be consistent with using 1-indexed indices and count and i, j will be 1-indexed too *)
@@ -98,11 +103,11 @@ let print_file ~range:((start_line, start_char), (end_line, end_char)) ~lines pp
98103
if current_line_strictly_between_start_and_end_line then
99104
fprintf ppf "%c@," current_char
100105
else if i = start_line then begin
101-
if j == (start_char - columns_to_cut) then fprintf ppf "@{<affected_line_content>";
106+
if j == (start_char - columns_to_cut) then fprintf ppf highlighted_content;
102107
fprintf ppf "%c@," current_char;
103108
if j == (end_char - columns_to_cut) then fprintf ppf "@}"
104109
end else if i = end_line then begin
105-
if j == 1 then fprintf ppf "@{<affected_line_content>";
110+
if j == 1 then fprintf ppf highlighted_content;
106111
fprintf ppf "%c@," current_char;
107112
if j == (end_char - columns_to_cut) then fprintf ppf "@}"
108113
end else
@@ -120,11 +125,9 @@ let print_file ~range:((start_line, start_char), (end_line, end_char)) ~lines pp
120125
done;
121126
fprintf ppf "@]" (* v *)
122127

123-
(* This allows you to, just like css, define which tag gets colored in which
124-
way. See usage in e.g. Super_location.super_error_reporter *)
125-
let colorize_tagged_string ppf f =
128+
let setup_colors ppf =
126129
Format.pp_set_formatter_tag_functions ppf
127130
({ (Format.pp_get_formatter_tag_functions ppf () ) with
128-
mark_open_tag = Ext_color.ansi_of_tag ~style_of_tag:f;
131+
mark_open_tag = Ext_color.ansi_of_tag;
129132
mark_close_tag = (fun _ -> Ext_color.reset_lit);
130133
})

jscomp/super_errors/super_misc.mli

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
(** Range coordinates all 1-indexed, like for editors. Otherwise this code
22
would have way too many off-by-one errors *)
3-
val print_file: range:(int * int) * (int * int) -> lines:string array -> Format.formatter -> unit -> unit
3+
val print_file: is_warning:bool -> range:(int * int) * (int * int) -> lines:string array -> Format.formatter -> unit -> unit
44

5-
(** This will be called in super_main. This is how you override the default error and warning printers *)
6-
val colorize_tagged_string: Format.formatter -> (string -> Ext_color.style list) -> unit
5+
val setup_colors: Format.formatter -> unit

jscomp/super_errors/super_typecore.ml

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,9 @@ let report_error env ppf = function
4848
(* modified *)
4949
report_unification_error ppf env trace
5050
(function ppf ->
51-
fprintf ppf "@{<Expr_type_clash.this>This is:@}")
51+
fprintf ppf "@{<error>This is:@}")
5252
(function ppf ->
53-
fprintf ppf "@{<Expr_type_clash.wanted>but somewhere wanted:@}")
53+
fprintf ppf "@{<info>but somewhere wanted:@}")
5454
| Apply_non_function typ ->
5555
(* modified *)
5656
reset_and_mark_loops typ;
@@ -233,11 +233,7 @@ let report_error env ppf = function
233233

234234
(* https://github.com/ocaml/ocaml/blob/4.02/typing/typecore.ml#L3979 *)
235235
let report_error env ppf err =
236-
Super_misc.colorize_tagged_string ppf (function
237-
| "Expr_type_clash.this" -> [Bold; FG Red]
238-
| "Expr_type_clash.wanted" -> [Bold; FG Yellow]
239-
| _ -> []
240-
);
236+
Super_misc.setup_colors ppf;
241237
wrap_printing_env env (fun () -> report_error env ppf err)
242238

243239
(* This will be called in super_main. This is how you'd override the default error printer from the compiler & register new error_of_exn handlers *)

0 commit comments

Comments
 (0)