From 28a2728c8f2c19ac7aeb65003e86e088525e9817 Mon Sep 17 00:00:00 2001 From: Kento Okura Date: Sat, 9 May 2026 09:18:53 +0200 Subject: [PATCH] Fix a variety of issues with the language server - Handle an edge case when an editor opens a buffer with no corresponding file. This caused grace to raise an error, as it attempted to index into the file. - Implement a cram test for the language server. This uses neovim in headless mode and a lua script that opens a file with errors and verifies that a diagnostic is returned. In the future this can be augmented to verify that other handlers work as well. - Driver.language_server no longer exits when done. - Remove source_path argument from Eval.eval_tree. Sources are kept track of in the tree itself. Dev mode behavior (rendering source paths) should be handled at render time instead of by the presence or lack of an optional value. - Fix a bug in Phases.parse. Now it correctly tries to parse a document. - Fix a bug in Lsp_shims that caused a crash: https://github.com/johnyob/grace/issues/90 - The changes in the test output of queries.t is explained by the fact that some rendering code was changed, thus the hash that is used to generate ids also changed. --- TODO.md | 2 +- lib/compiler/Driver.ml | 24 ++++++------- lib/compiler/Eval.ml | 11 +++--- lib/compiler/Phases.ml | 24 ++++++------- lib/compiler/State.ml | 1 + lib/core/Tree.ml | 18 ++++++++-- lib/frontend/Html_client.ml | 8 ++--- lib/frontend/templates/Page.ml | 10 +++--- lib/frontend/test/config.t | 1 - lib/frontend/test/configs/trees/index.tree | 0 lib/language_server/Did_change.ml | 4 ++- lib/language_server/Did_create_files.ml | 2 +- lib/language_server/Did_open.ml | 10 ++++-- lib/language_server/Lsp_shims.ml | 40 ++++++++++------------ lib/parser/Parse.ml | 6 +++- lib/server/Server.ml | 6 ++-- test/dune | 11 +++--- test/lsp.t | 20 +++++++++++ test/lsp_driver.lua | 30 ++++++++++++++++ test/queries.t | 2 +- 20 files changed, 153 insertions(+), 77 deletions(-) create mode 100644 lib/frontend/test/configs/trees/index.tree create mode 100644 test/lsp.t create mode 100644 test/lsp_driver.lua diff --git a/TODO.md b/TODO.md index 9af6b28..d74252e 100644 --- a/TODO.md +++ b/TODO.md @@ -13,7 +13,7 @@ once they are fixed or otherwise invalidated. - [ ] \transclude/toc flag within transcluded tree is not respected in parent. For example, my blog page (https://www.jonmsterling.com/007W/) rightly doesn't have a table of contents, but my home page (which transcludes the blog) does. -- [ ] Language server crashes upon opening an editor (Neovim, Helix). This doesn't happen if I run the language server from the command line, so I think the crash might be within the handshake or something. +- [x] Language server crashes upon opening an editor (Neovim, Helix). This doesn't happen if I run the language server from the command line, so I think the crash might be within the handshake or something. - [x] Spurious "duplicate tree" warnings with a LaTeX-generated SVG file. diff --git a/lib/compiler/Driver.ml b/lib/compiler/Driver.ml index 8a4e7c0..91d7d4e 100644 --- a/lib/compiler/Driver.ml +++ b/lib/compiler/Driver.ml @@ -12,11 +12,11 @@ open struct module T = Types end -let update (action : Action.t) (forest : State.t) = +let update ?(bail = true) (action : Action.t) (forest : State.t) = Action.log action; match action with - | Quit Fail -> exit 1 - | Quit Finished -> exit 0 + | Quit Fail -> if bail then exit 1 else (Done, forest) + | Quit Finished -> if bail then exit 0 else (Done, forest) | Query q -> let r = Forest.run_datalog_query forest.graphs q in (Query_results r, forest) @@ -77,12 +77,12 @@ let update (action : Action.t) (forest : State.t) = (Phases.parse forest uri, forest) | Done -> (Done, forest) -let run_until_done a s : State.t = +let run_until_done ?(bail = true) a s : State.t = let rec go action state = - let new_action, new_state = update action state in + let new_action, new_state = update ~bail action state in match action with - | Quit Fail -> exit 1 - | Quit Finished -> exit 0 + | Quit Fail -> new_state + | Quit Finished -> new_state | Done -> new_state | _ -> go new_action new_state in @@ -114,12 +114,12 @@ let batch_run ~env ~(config : Config.t) ~dev = let language_server ~env ~config = let init = State.make ~env ~config ~dev:true () in let rec go action state = - let new_action, new_state = update action state in match action with - | Quit Fail -> exit 1 - | Quit Finished -> exit 0 - | Done -> new_state - | _ -> go new_action new_state + | Quit Fail | Quit Finished -> state + | Done -> state + | _ -> + let new_action, new_state = update ~bail:false action state in + go new_action new_state in let _ = update Plant_assets init in go Load_configured_dirs init diff --git a/lib/compiler/Eval.ml b/lib/compiler/Eval.ml index db21cf9..d19a4c1 100644 --- a/lib/compiler/Eval.ml +++ b/lib/compiler/Eval.ml @@ -790,19 +790,22 @@ and eval_tree_inner ~env ~range ?(uri : URI.t option) (syn : Syn.t) : let eval_tree : config:Config.t -> uri:URI.t -> - source_path:string option -> Tree.(expanded tree) -> eval_result option * Error.t list = - fun ~config ~uri ~source_path tree -> + fun ~config ~uri tree -> let res = ref None in + let source_path = + match tree.source with + | `String {name = _; _} -> None + | `File path -> Some path + in let errors = let@ () = Error.collect in let fm = T.default_frontmatter ~uri ?source_path () in let env = initial_eval_env config (ref fm) in - let source = Tree.Expanded.source tree in let nodes = Tree.Expanded.nodes tree in let range = - Range.(fun source -> total ~source) (source :> Grace.Source.t) + Range.(fun source -> total ~source) (tree.source :> Grace.Source.t) in match eval_tree_inner ~range ~env ~uri nodes with | Error error -> Error.yield_eval_error error diff --git a/lib/compiler/Phases.ml b/lib/compiler/Phases.ml index e0e3d1d..3655e2c 100644 --- a/lib/compiler/Phases.ml +++ b/lib/compiler/Phases.ml @@ -67,11 +67,15 @@ let parse (forest : State.t) uri = forest.?{uri} <- errors; report ~errors ~and_then:Done end - ~none:begin match State.get_code ~forest uri with - | Some code -> - Imports.fixup ~uri code forest; - forest.={uri} <- Tree code; - Expand uri + ~none:begin match State.get_document ~forest uri with + | Some doc -> begin + match Parse.parse_document doc with + | Error e -> report ~errors:[Error.parse_error e] ~and_then:Done + | Ok code -> + Imports.fixup ~uri code forest; + forest.={uri} <- Tree code; + Expand uri + end | None -> assert false end @@ -177,10 +181,7 @@ let eval (forest : State.t) = organize @@ let@ uri, tree = List.map @~ expanded in - let source_path = - if State.dev forest then Option.map Tree.path forest.={uri} else None - in - Eval.(eval_tree ~config ~source_path ~uri tree) + Eval.(eval_tree ~config ~uri tree) in let () = let@ article = List.iter @~ articles in @@ -196,10 +197,7 @@ let eval_only (uri : URI.t) (forest : State.t) = match phase with | Loaded | Parsed | Evaluated -> assert false | Expanded -> begin - let source_path = - if State.dev forest then Option.map Tree.path forest.={uri} else None - in - let result, errors = Eval.eval_tree ~config ~source_path ~uri t in + let result, errors = Eval.eval_tree ~config ~uri t in forest.?{uri} <- errors; match result with | None -> (errors, []) diff --git a/lib/compiler/State.ml b/lib/compiler/State.ml index 6c2413c..4fcc1c0 100644 --- a/lib/compiler/State.ml +++ b/lib/compiler/State.ml @@ -120,6 +120,7 @@ open Syntax let get_resource ~forest uri = Option.bind forest.={uri} Tree.resource let get_code ~forest uri = Option.bind forest.={uri} Tree.code +let get_document ~forest uri = Option.bind forest.={uri} Tree.document let get_article ~forest uri = Option.bind forest.={uri} Tree.article let section_symbol = "ยง" diff --git a/lib/core/Tree.ml b/lib/core/Tree.ml index bba3637..b0c453d 100644 --- a/lib/core/Tree.ml +++ b/lib/core/Tree.ml @@ -12,7 +12,13 @@ open struct include Base end -type source = [`File of string] +type source = [`File of string | `String of Grace.Source.string_source] +let show_source : source -> string = function + | `File string -> "file: " ^ string + | `String {name; _} -> + Option.fold name + ~some:(fun s -> "String " ^ s) + ~none:"" type exports = (R.P.data, Grace.Range.t option) Trie.t @@ -79,7 +85,10 @@ type phase = Phase : 'a tag -> phase let source : t -> _ = function Tree {source; _} -> source -let path = function Tree {source = `File path; _} -> path +let path = function + | Tree {source = `File path; _} -> path + | Tree {source = `String {name; _}; _} -> + Option.value name ~default:"" let phase : type a. a tree -> phase = function | {phase; _} -> begin @@ -104,7 +113,10 @@ let lsp_uri : t -> Lsp.Uri.t = function match phase with | Loaded -> Lsp.Text_document.documentUri tree | Parsed | Expanded | Evaluated -> begin - match source with `File string -> Lsp.Uri.of_string string + match source with + | `File string -> Lsp.Uri.of_string string + | `String {name; _} -> + Option.fold name ~some:Lsp.Uri.of_string ~none:(failwith "Tree.lsp_uri") end end diff --git a/lib/frontend/Html_client.ml b/lib/frontend/Html_client.ml index bddea3c..2dbaa10 100644 --- a/lib/frontend/Html_client.ml +++ b/lib/frontend/Html_client.ml @@ -492,14 +492,14 @@ and render_display_uri ~env ?(slug = false) uri : P.node = ] [P.txt "["; P.txt "%s" path_string; P.txt "]"] -and render_source_path (frontmatter : T.(content frontmatter)) = +and render_source_path ~(env : env) (frontmatter : T.(content frontmatter)) = (* TODO: Check dev mode *) match frontmatter.source_path with - | None -> H.null [] - | Some source_path -> + | Some source_path when env.forest.dev -> H.a [H.class_ "edit-button"; H.href "vscode://file%s" source_path] [P.txt "[edit]"] + | _ -> H.null [] and render_frontmatter ~env (frontmatter : _ T.frontmatter) : P.node = H.header [] @@ -513,7 +513,7 @@ and render_frontmatter ~env (frontmatter : _ T.frontmatter) : P.node = P.txt " "; render_display_uri ~env ~slug:true frontmatter.uri; P.txt " "; - render_source_path frontmatter; + render_source_path ~env frontmatter; ]; H.div [H.class_ "metadata"] diff --git a/lib/frontend/templates/Page.ml b/lib/frontend/templates/Page.ml index 6ecdac8..29a5eed 100644 --- a/lib/frontend/templates/Page.ml +++ b/lib/frontend/templates/Page.ml @@ -28,8 +28,8 @@ let page_header ~is_home ~dev = ]; ] -let v ?(dev = false) ?(is_home = false) ?(title_string = "") ?(mode = Static) - ?(source_path = None) ?(render_header = false) c = +let v ~dev ?(is_home = false) ?(title_string = "") ?(source_path = None) + ?(render_header = false) ~mode c = let open H in html [] [ @@ -43,8 +43,10 @@ let v ?(dev = false) ?(is_home = false) ?(title_string = "") ?(mode = Static) (match mode with | Dynamic -> H.null [script [type_ "module"; src "/htmx.js"] ""] | Static -> H.null []); - optional source_path (fun path -> - script [type_ "module"] "window.sourcePath = '%s'" path); + (if dev then + optional source_path (fun path -> + script [type_ "module"] "window.sourcePath = '%s'" path) + else H.null []); script [type_ "module"; src "/min.js"] ""; title [] "%s" title_string; ]; diff --git a/lib/frontend/test/config.t b/lib/frontend/test/config.t index 75107d1..a1bf37b 100644 --- a/lib/frontend/test/config.t +++ b/lib/frontend/test/config.t @@ -4,7 +4,6 @@ warning: option [forest.trees] not set, using default [trees] warning: option [forest.assets] not set, using default [] warning: option [forest.home] not set, using default index - error[config_error]: directory trees does not exist $ parse_forester_config configs/uninterpreted-fields.toml warning[config_error]: unknown options forest.unknown diff --git a/lib/frontend/test/configs/trees/index.tree b/lib/frontend/test/configs/trees/index.tree new file mode 100644 index 0000000..e69de29 diff --git a/lib/language_server/Did_change.ml b/lib/language_server/Did_change.ml index bca87e7..dcda2dc 100644 --- a/lib/language_server/Did_change.ml +++ b/lib/language_server/Did_change.ml @@ -30,6 +30,8 @@ let compute (params : L.DidChangeTextDocumentParams.t) = in forest.={uri} <- Tree updated; Lsp_state.modify (fun ({forest; _} as lsp_state) -> - let new_forest = Driver.run_until_done (Action.Parse lsp_uri) forest in + let new_forest = + Driver.run_until_done ~bail:false (Action.Parse lsp_uri) forest + in {lsp_state with forest = new_forest}); Diagnostics.compute updated.tree diff --git a/lib/language_server/Did_create_files.ml b/lib/language_server/Did_create_files.ml index aed9fe0..bb0a0ca 100644 --- a/lib/language_server/Did_create_files.ml +++ b/lib/language_server/Did_create_files.ml @@ -25,5 +25,5 @@ let compute ({files} : L.CreateFilesParams.t) = let doc = Phases.load_tree path in forest.={uri} <- Tree doc end; - let new_forest = Driver.run_until_done Parse_all forest in + let new_forest = Driver.run_until_done ~bail:false Parse_all forest in {lsp_state with forest = new_forest} diff --git a/lib/language_server/Did_open.ml b/lib/language_server/Did_open.ml index 93f27d9..33b6d9f 100644 --- a/lib/language_server/Did_open.ml +++ b/lib/language_server/Did_open.ml @@ -19,8 +19,14 @@ let compute (params : L.DidOpenTextDocumentParams.t) = let Lsp_state.{forest; _} = Lsp_state.get () in let document = Lsp.Text_document.make ~position_encoding:`UTF16 params in let uri = URI.of_lsp_uri ~base:forest.config.url lsp_uri in - forest.={uri} <- Tree {tree = document; source = `File path; phase = Loaded}; + let source : Tree.source = + if try Sys.is_regular_file path with _ -> false then `File path + else `String {name = Some path; content = Lsp.Text_document.text document} + in + forest.={uri} <- Tree (Tree.Loaded.create ~source document); Lsp_state.modify (fun ({forest; _} as lsp_state) -> - let new_forest = Driver.run_until_done (Action.Parse lsp_uri) forest in + let new_forest = + Driver.run_until_done ~bail:false (Action.Parse lsp_uri) forest + in {lsp_state with forest = new_forest}); Diagnostics.compute document diff --git a/lib/language_server/Lsp_shims.ml b/lib/language_server/Lsp_shims.ml index fcc9587..6c56db5 100644 --- a/lib/language_server/Lsp_shims.ml +++ b/lib/language_server/Lsp_shims.ml @@ -9,44 +9,40 @@ open struct module L = Lsp.Types end +open Forester_core open Grace +module L = Lsp.Types + let lsp_pos_of_range (range : Range.t) : L.Position.t = - let source = Grace_source_reader.open_source @@ Range.source range in + let open Grace_source_reader in + let source = open_source @@ Range.source range in let start, stop = Range.split range in - let start_line = Grace_source_reader.Line.of_byte_index source start in + let start_line = Line.of_byte_index source start in let line = (start_line.idx :> int) in - let line_start = Grace_source_reader.Line.start start_line in - let character = - Grace_source_reader.(slicei source line_start start) |> String.length - in + let line_start = Line.start start_line in + let character = slicei source line_start start |> String.length in L.Position.create ~line ~character let lsp_range_of_range (range : Range.t) : L.Range.t = - let source = Grace_source_reader.open_source @@ Range.source range in + let open Grace_source_reader in + Grace_source_reader.with_reader @@ fun () -> + let source = open_source @@ Range.source range in let start, stop = Range.split range in - - let start_line = Grace_source_reader.Line.of_byte_index source start in - let stop_line = Grace_source_reader.Line.of_byte_index source stop in - + let start_line = Line.of_byte_index source start in + let stop_line = Line.of_byte_index source stop in let start = - let line = (start_line.idx :> int) in - let line_start = Grace_source_reader.Line.start start_line in - let character = - Grace_source_reader.(slicei source line_start start) |> String.length - in + let line = (start_line.idx :> int) + 1 in + let line_start = Line.start start_line in + let character = Line.column_offset ~in_:source start_line start in L.Position.create ~line ~character in - let end_ = let line = (stop_line.idx :> int) in - let line_start = Grace_source_reader.Line.start stop_line in - let character = - Grace_source_reader.(slicei source line_start stop) |> String.length - in + let line_start = Line.start stop_line in + let character = slicei source line_start stop |> String.length in L.Position.create ~line ~character in - L.Range.create ~start ~end_ let location_of_range loc = diff --git a/lib/parser/Parse.ml b/lib/parser/Parse.ml index 7f263d9..23b6b80 100644 --- a/lib/parser/Parse.ml +++ b/lib/parser/Parse.ml @@ -55,10 +55,14 @@ let parse_document doc : (Tree.(parsed tree), _) result = let uri = Lsp.Text_document.documentUri doc in let path = Lsp.Uri.to_path uri in let text = Lsp.Text_document.text doc in + let source : Tree.source = + if try Sys.is_regular_file path with _ -> false then `File path + else `String {name = Some path; content = text} + in let lexbuf = Lexing.from_string text in lexbuf.lex_curr_p <- {lexbuf.lex_curr_p with pos_fname = path}; parse (`String {content = text; name = Some path}) lexbuf - |> Result.map (Tree.Parsed.create ~source:(`File path)) + |> Result.map (Tree.Parsed.create ~source) let parse_file filename = let ch = open_in filename in diff --git a/lib/server/Server.ml b/lib/server/Server.ml index 24cd3f3..ea89d3a 100644 --- a/lib/server/Server.ml +++ b/lib/server/Server.ml @@ -240,14 +240,16 @@ let handler : match route with | Json_manifest -> respond_json ~status:`OK ~body:(`List []) () | _ -> - let body = Templates.(page ~mode:Dynamic [no_config_found]) in + let body = + Templates.(page ~mode:Dynamic ~dev:true [no_config_found]) + in respond_html ~body ~status:`OK () end | Some forest -> begin match route with | Static _ | Config -> assert false | Index -> - let body = Templates.index in + let body = Templates.index ~mode:Dynamic ~dev:true in respond_html ~status:`OK ~body () | Json_manifest -> respond_json ~status:`OK ~body:(`List []) () | Tree uri -> tree_handler ~forest ~request_headers uri diff --git a/test/dune b/test/dune index 1572f78..b206985 100644 --- a/test/dune +++ b/test/dune @@ -30,11 +30,12 @@ yojson)) (cram - (deps - %{bin:forester} - %{bin:print-syn} - %{bin:print-code} - (glob_files_rec forests/*))) + (deps %{bin:forester} %{bin:print-syn} %{bin:print-code})) + +(cram + (applies_to lsp) + (deps %{bin:forester} lsp_driver.lua) + (enabled_if %{bin-available:nvim})) (executables (names Print_syn Print_code) diff --git a/test/lsp.t b/test/lsp.t new file mode 100644 index 0000000..e3bb627 --- /dev/null +++ b/test/lsp.t @@ -0,0 +1,20 @@ + $ forester init + note: Created directory trees + note: Created directory assets + note: Created file forest.toml + note: Created file .gitignore + note: Created file assets/.gitkeep + note: Created file trees/index.tree + note: Initialized forest, try editing `trees/index.tree` and running `forester build`. Afterwards, you can open `output/index.html` in your browser to view your forest. + + $ cat > trees/index.tree << EOF + > \title{hello + > EOF + + $ nvim --headless --clean -l lsp_driver.lua + { + column = 0, + line = 2, + message = "parse error", + severity = 1 + } diff --git a/test/lsp_driver.lua b/test/lsp_driver.lua new file mode 100644 index 0000000..8eaad60 --- /dev/null +++ b/test/lsp_driver.lua @@ -0,0 +1,30 @@ +vim.lsp.log._set_filename("./lsp.log") + +vim.filetype.add({ extension = { tree = "forester" } }) + +vim.api.nvim_create_autocmd("FileType", { + pattern = "forester", + group = vim.api.nvim_create_augroup("start_lsp", { clear = true }), + callback = function() + vim.lsp.start({ + name = "forester", + cmd = { "forester", "lsp", "-vvv" }, + root_dir = vim.fn.getcwd(), + }) + end, +}) + +local bufnr = vim.fn.bufadd("trees/index.tree") +vim.bo[bufnr].filetype = "forester" +vim.fn.bufload(bufnr) + +vim.wait(3000, function() + return #vim.diagnostic.get(bufnr) > 0 +end, 100) + +local diags = vim.diagnostic.get(bufnr) + +for _, diag in pairs(diags) do + local d = { severity = diag.severity, line = diag.lnum, column = diag.col, message = diag.message } + vim.print(d) +end diff --git a/test/queries.t b/test/queries.t index debd911..29a5dde 100644 --- a/test/queries.t +++ b/test/queries.t @@ -79,7 +79,7 @@
-
+

Reference Some novel result [paper]

-- 2.51.2