Skip to content

Commit 55e6e68

Browse files
committed
[Super errors] Remove dependency on Format for the code frame display
This is a mostly feature-compatible refactor of super_misc, which is used to display the code frame in super errors reporting. I've optimized, and added useless features, to keep the tests mostly still intact so that this can be merged more comfortably. After this gets merged, I'll clean up some more logic. Changes: - No more Format. - No more need to patch compiler for colors (will do this in the next diffs). - Less need to understand the intricacies of coloring. It's now mostly encapsulate within this module.
1 parent 0e8edb5 commit 55e6e68

File tree

5 files changed

+304
-182
lines changed

5 files changed

+304
-182
lines changed
Lines changed: 280 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,280 @@
1+
2+
let digits_count n =
3+
let rec loop n base count =
4+
if n >= base then loop n (base * 10) (count + 1) else count
5+
in
6+
loop (abs n) 1 0
7+
8+
let seek_2_lines_before src pos =
9+
let open Lexing in
10+
let original_line = pos.pos_lnum in
11+
let rec loop current_line current_char =
12+
if current_line + 2 >= original_line then
13+
(current_char, current_line)
14+
else
15+
loop
16+
(if src.[current_char] = '\n' then current_line + 1 else current_line)
17+
(current_char + 1)
18+
in
19+
loop 1 0
20+
21+
let seek_2_lines_after src pos =
22+
let open Lexing in
23+
let original_line = pos.pos_lnum in
24+
let rec loop current_line current_char =
25+
if current_char = String.length src then
26+
(current_char, current_line)
27+
else
28+
match src.[current_char] with
29+
| '\n' when current_line = original_line + 2 ->
30+
(current_char, current_line)
31+
| '\n' -> loop (current_line + 1) (current_char + 1)
32+
| _ -> loop current_line (current_char + 1)
33+
in
34+
loop original_line pos.pos_cnum
35+
36+
let leading_space_count str =
37+
let rec loop i count =
38+
if i = String.length str then count
39+
else if str.[i] != ' ' then count
40+
else loop (i + 1) (count + 1)
41+
in
42+
loop 0 0
43+
44+
let break_long_line max_width line =
45+
let rec loop pos accum =
46+
if pos = String.length line then accum
47+
else
48+
let chunk_length = min max_width (String.length line - pos) in
49+
let chunk = String.sub line pos chunk_length in
50+
loop (pos + chunk_length) (chunk::accum)
51+
in
52+
loop 0 [] |> List.rev
53+
54+
let filter_mapi f l =
55+
let rec loop f l i accum =
56+
match l with
57+
| [] -> accum
58+
| head::rest ->
59+
let accum =
60+
match f i head with
61+
| None -> accum
62+
| Some result -> result::accum
63+
in
64+
loop f rest (i + 1) accum
65+
in
66+
loop f l 0 [] |> List.rev
67+
68+
type color =
69+
| Dim
70+
(* | Filename *)
71+
| Err
72+
| Warn
73+
| NoColor
74+
75+
let dim = "\x1b[2m"
76+
(* let filename = "\x1b[46m" *)
77+
let err = "\x1b[1;31m"
78+
let warn = "\x1b[1;33m"
79+
let reset = "\x1b[0m"
80+
81+
external isatty : out_channel -> bool = "caml_sys_isatty"
82+
(* reasonable heuristic on whether colors should be enabled *)
83+
let should_enable_color () =
84+
let term = try Sys.getenv "TERM" with Not_found -> "" in
85+
term <> "dumb"
86+
&& term <> ""
87+
&& isatty stderr
88+
89+
let color_enabled = ref true
90+
91+
let setup =
92+
let first = ref true in (* initialize only once *)
93+
fun o ->
94+
if !first then (
95+
first := false;
96+
color_enabled := (match o with
97+
| Some Misc.Color.Always -> true
98+
| Some Auto -> should_enable_color ()
99+
| Some Never -> false
100+
| None -> should_enable_color ())
101+
);
102+
()
103+
104+
(* external isatty : out_channel -> bool = "caml_sys_isatty"
105+
106+
let should_enable_color =
107+
let term = try Sys.getenv "TERM" with Not_found -> "" in
108+
term <> "dumb"
109+
&& term <> ""
110+
&& isatty stderr
111+
*)
112+
let last_color = ref NoColor
113+
let col color str =
114+
if not !color_enabled then str
115+
else begin
116+
let s = match !last_color, color with
117+
| c1, c2 when c1 = c2 -> str
118+
| NoColor, Dim -> dim ^ str
119+
(* | NoColor, Filename -> filename ^ str *)
120+
| NoColor, Err -> err ^ str
121+
| NoColor, Warn -> warn ^ str
122+
| _, NoColor -> reset ^ str
123+
| _, Dim -> reset ^ dim ^ str
124+
(* | _, Filename -> reset ^ filename ^ str *)
125+
| _, Err -> reset ^ err ^ str
126+
| _, Warn -> reset ^ warn ^ str
127+
in
128+
last_color := color;
129+
s
130+
end
131+
132+
type gutter = Number of int | Elided
133+
type highlighted_string = {s: string; start: int; end_: int}
134+
type line = {
135+
gutter: gutter;
136+
content: highlighted_string list;
137+
}
138+
(*
139+
Features:
140+
- display a line gutter
141+
- break long line into multiple for terminal display
142+
- peek 2 lines before & after for context
143+
- center snippet when it's heavily indented
144+
- ellide intermediate lines when the reported range is huge
145+
*)
146+
let print ~is_warning ~src ~startPos ~endPos =
147+
let open Lexing in
148+
149+
let indent = 2 in
150+
let highlight_line_start_line = startPos.pos_lnum in
151+
let highlight_line_end_line = endPos.pos_lnum in
152+
let (start_line_line_offset, first_shown_line) = seek_2_lines_before src startPos in
153+
let (end_line_line_end_offset, last_shown_line) = seek_2_lines_after src endPos in
154+
155+
let more_than_5_highlighted_lines =
156+
highlight_line_end_line - highlight_line_start_line + 1 > 5
157+
in
158+
let max_line_digits_count = digits_count last_shown_line in
159+
(* TODO: change this back to a fixed 100? *)
160+
(* 3 for separator + the 2 spaces around it *)
161+
let line_width = 78 - max_line_digits_count - indent - 3 in
162+
let lines =
163+
(* TODO: off-by-one danger *)
164+
String.sub src start_line_line_offset (end_line_line_end_offset - start_line_line_offset)
165+
in
166+
(* TODO: remove this after the next PR *)
167+
let len = String.length lines in
168+
let lines =
169+
if len > 1 && (String.get src (len - 1)) = '\n' then
170+
String.sub lines 0 (len - 1)
171+
else
172+
lines
173+
in
174+
let lines = lines
175+
|> String.split_on_char '\n'
176+
|> filter_mapi (fun i line ->
177+
let line_number = i + first_shown_line in
178+
if more_than_5_highlighted_lines then
179+
if line_number = highlight_line_start_line + 2 then
180+
Some (Elided, line)
181+
else if line_number > highlight_line_start_line + 2 && line_number < highlight_line_end_line - 1 then None
182+
else Some (Number line_number, line)
183+
else Some (Number line_number, line)
184+
)
185+
in
186+
let leading_space_to_cut = lines |> List.fold_left (fun current_max (_, line) ->
187+
let leading_spaces = leading_space_count line in
188+
if String.length line = leading_spaces then
189+
(* the line's nothing but spaces. Doesn't count *)
190+
current_max
191+
else
192+
min leading_spaces current_max
193+
) 99999
194+
in
195+
let separator = if leading_space_to_cut = 0 then "" else "" in
196+
let stripped_lines = lines |> List.map (fun (gutter, line) ->
197+
let new_content =
198+
if String.length line <= leading_space_to_cut then
199+
[{s = ""; start = 0; end_ = 0}]
200+
else
201+
String.sub line leading_space_to_cut (String.length line - leading_space_to_cut)
202+
|> break_long_line line_width
203+
|> List.mapi (fun i line ->
204+
match gutter with
205+
| Elided -> {s = line; start = 0; end_ = 0}
206+
| Number line_number ->
207+
let highlight_line_start_offset = startPos.pos_cnum - startPos.pos_bol in
208+
let highlight_line_end_offset = endPos.pos_cnum - endPos.pos_bol in
209+
let start =
210+
if i = 0 && line_number = highlight_line_start_line then
211+
highlight_line_start_offset - leading_space_to_cut
212+
else 0
213+
in
214+
let end_ =
215+
if line_number < highlight_line_start_line then 0
216+
else if line_number = highlight_line_start_line && line_number = highlight_line_end_line then
217+
highlight_line_end_offset - leading_space_to_cut
218+
else if line_number = highlight_line_start_line then
219+
String.length line
220+
else if line_number > highlight_line_start_line && line_number < highlight_line_end_line then
221+
String.length line
222+
else if line_number = highlight_line_end_line then highlight_line_end_offset - leading_space_to_cut
223+
else 0
224+
in
225+
{s = line; start; end_}
226+
)
227+
in
228+
{gutter; content = new_content}
229+
)
230+
in
231+
let buf = Buffer.create 100 in
232+
let draw_gutter color s =
233+
(* TODO: simplify after the next PR *)
234+
let pad = String.make (max_line_digits_count + indent - String.length s) ' ' in
235+
(* TODO: encapstulate adding string/char *)
236+
Buffer.add_string buf (col NoColor pad);
237+
Buffer.add_string buf (col color s);
238+
Buffer.add_string buf (col NoColor " ");
239+
Buffer.add_string buf (col Dim separator);
240+
Buffer.add_string buf (col NoColor " ");
241+
in
242+
stripped_lines |> List.iter (fun {gutter; content} ->
243+
match gutter with
244+
| Elided ->
245+
draw_gutter Dim ".";
246+
Buffer.add_string buf (col Dim "...");
247+
(* TODO: remove this after the next PR *)
248+
Buffer.add_string buf (col NoColor "\n");
249+
| Number line_number -> begin
250+
content |> List.iteri (fun i line ->
251+
if i = 0 then begin
252+
let gutter_color =
253+
if i = 0
254+
&& line_number >= highlight_line_start_line
255+
&& line_number <= highlight_line_end_line then
256+
if is_warning then Warn else Err
257+
else NoColor
258+
in
259+
draw_gutter gutter_color (string_of_int line_number);
260+
end else begin
261+
(* TODO: remove this branch after the next PR *)
262+
let pad = String.make (max_line_digits_count + indent + 3) ' ' in
263+
Buffer.add_string buf (col NoColor pad);
264+
end;
265+
266+
line.s |> String.iteri (fun ii ch ->
267+
let c =
268+
if ii >= line.start && ii < line.end_ then
269+
if is_warning then Warn else Err
270+
else NoColor in
271+
Buffer.add_string buf (col c (String.make 1 ch));
272+
);
273+
Buffer.add_string buf (col NoColor "\n");
274+
);
275+
end
276+
);
277+
(* TODO: remove the extra space that catered to making existing tests pass *)
278+
Buffer.add_string buf (col NoColor " ");
279+
Buffer.contents buf
280+
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
val print: is_warning:bool -> src:string -> startPos:Lexing.position -> endPos:Lexing.position -> string
2+
3+
val setup : Misc.Color.setting option -> unit
4+
(* [setup opt] will enable or disable color handling for print
5+
according to the value of color setting [opt].
6+
Only the first call to this function has an effect. *)

jscomp/super_errors/super_location.ml

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
11
let fprintf = Format.fprintf
22

3-
let file_lines filePath =
4-
Ext_array.reverse_of_list
5-
(Ext_io.rev_lines_of_file filePath)
6-
73
let setup_colors () =
8-
Misc.Color.setup !Clflags.color
4+
Misc.Color.setup !Clflags.color;
5+
Super_code_frame.setup !Clflags.color
96

107
let print_filename = Location.print_filename
118

@@ -54,18 +51,24 @@ let print ~message_kind intro ppf (loc : Location.t) =
5451
(* again: end_char is exclusive, so +1-1=0 *)
5552
Some ((start_line, start_char + 1), (end_line, end_char))
5653
in
57-
fprintf ppf "@[%a@]@," (print_loc ~normalizedRange) loc;
54+
(* TODO: remove the extra space that catered to making existing tests pass *)
55+
fprintf ppf " @[%a@]@, " (print_loc ~normalizedRange) loc;
56+
(* TODO: clean up *)
5857
match normalizedRange with
5958
| None -> ()
60-
| Some range -> begin
59+
| Some _ -> begin
6160
try
62-
let lines = file_lines file in
61+
let src = Ext_io.load_file file in
6362
(* we're putting the line break `@,` here rather than above, because this
6463
branch might not be reached (aka no inline file content display) so
6564
we don't wanna end up with two line breaks in the the consequent *)
66-
fprintf ppf "@,%a"
67-
(Super_misc.print_file ~is_warning:(message_kind=`warning) ~lines ~range)
68-
()
65+
fprintf ppf "@,%s"
66+
(Super_code_frame.print
67+
~is_warning:(message_kind=`warning)
68+
~src
69+
~startPos:loc.loc_start
70+
~endPos:loc.loc_end
71+
)
6972
with
7073
(* this might happen if the file is e.g. "", "_none_" or any of the fake file name placeholders.
7174
we've already printed the location above, so nothing more to do here. *)
@@ -78,7 +81,8 @@ let print ~message_kind intro ppf (loc : Location.t) =
7881
let rec super_error_reporter ppf ({loc; msg; sub} : Location.error) =
7982
setup_colors ();
8083
(* open a vertical box. Everything in our message is indented 2 spaces *)
81-
Format.fprintf ppf "@[<v 2>@,%a@,%s@,@]" (print ~message_kind:`error "We've found a bug for you!") loc msg;
84+
(* TODO: clean up after next PR *)
85+
Format.fprintf ppf "@[<v 0>@, %a@, %s@,@]" (print ~message_kind:`error "We've found a bug for you!") loc msg;
8286
List.iter (Format.fprintf ppf "@,@[%a@]" super_error_reporter) sub
8387
(* no need to flush here; location's report_exception (which uses this ultimately) flushes *)
8488

@@ -91,7 +95,8 @@ let super_warning_printer loc ppf w =
9195
| `Active { Warnings. number = _; message = _; is_error; sub_locs = _} ->
9296
setup_colors ();
9397
let message_kind = if is_error then `warning_as_error else `warning in
94-
Format.fprintf ppf "@[<v 2>@,%a@,%s@,@]@."
98+
(* TODO: clean up after next PR *)
99+
Format.fprintf ppf "@[<v 0>@, %a@, %s@, @]@."
95100
(print ~message_kind ("Warning number " ^ (Warnings.number w |> string_of_int)))
96101
loc
97102
(Warnings.message w);

0 commit comments

Comments
 (0)