From 60ac2a6b1f46d2ce99b0b3fdf3670d7326d3ebb9 Mon Sep 17 00:00:00 2001 From: Kento Okura Date: Sat, 15 Aug 2026 14:15:22 +0200 Subject: [PATCH] Defer loading of configuration in lsp Helix sets the root directory for the language server to the first parent directory containing a .git directory. In the case of the docs forest tracked in this repository, this caused a crash, since there is no forest.toml file in that directory. The did_open handler now uses the path of the opened file to search upwards for a valid config file. --- TODO.md | 2 +- bin/forester/main.ml | 14 ++++++++++---- lib/compiler/Config_parser.ml | 27 ++++++++++++++++++++++++++- lib/compiler/Config_parser.mli | 5 +++++ lib/compiler/Driver.ml | 10 ++++++---- lib/compiler/State.ml | 2 +- lib/core/Config.ml | 7 +++++-- lib/core/Config.mli | 2 ++ lib/frontend/test/Test_config.ml | 2 ++ lib/language_server/Did_open.ml | 29 +++++++++++++++++++++++++---- lib/language_server/Forester_lsp.ml | 20 ++++++++++++++++++-- 11 files changed, 101 insertions(+), 19 deletions(-) diff --git a/TODO.md b/TODO.md index 47b4791..801ceea 100644 --- a/TODO.md +++ b/TODO.md @@ -4,7 +4,7 @@ I'm beginning to evaluate the state of this branch for its eventually merger into main. I will collect here the bugs that I notice, which you can remove once they are fixed or otherwise invalidated. -- [ ] Language server crash: begin editing a file and then open the preview in your browser. Immediate crash. +- [x] Language server crash: begin editing a file and then open the preview in your browser. Immediate crash. - [ ] Feature request: in search panel rows, show the tree address. Should also be able to search by tree address. diff --git a/bin/forester/main.ml b/bin/forester/main.ml index f0b17fb..8e79512 100644 --- a/bin/forester/main.ml +++ b/bin/forester/main.ml @@ -280,10 +280,8 @@ let init_cmd ~env:env_ : unit Cmd.t = let info = Cmd.info "init" ~version ~doc ~man in Cmd.v info Term.(const (init ~env:env_) $ arg_dir) -let lsp ~env _ config port no_serve = - match Config_parser.parse_forest_config_file ~env config with - | Error errors -> List.iter Error.print_config_error errors - | Ok config -> Forester_lsp.start ~env ~port ~serve:(not no_serve) ~config +let lsp ~env _ config_path port no_serve = + Forester_lsp.start ~env ~port ~serve:(not no_serve) ~config_path let lsp_cmd ~env:env_ = let man = @@ -293,6 +291,14 @@ let lsp_cmd ~env:env_ = ] in let doc = "Start the LSP" in + let arg_config = + let doc = + "A TOML file like $(i,forest.toml). If omitted, the server locates it \ + automatically by searching upward from each opened document's directory \ + through its parent directories." + in + Arg.(value & pos 0 (some file) None & info [] ~docv:"FOREST" ~doc) + in let arg_port = let doc = "Port for the live HTTP preview served alongside the LSP" in Arg.(value & opt int 8080 & info ["p"; "port"] ~doc ~docv:"PORT") diff --git a/lib/compiler/Config_parser.ml b/lib/compiler/Config_parser.ml index 4e07efe..8aec462 100644 --- a/lib/compiler/Config_parser.ml +++ b/lib/compiler/Config_parser.ml @@ -132,7 +132,8 @@ let parse lexbuf filename : (Config.t, _ Config_error.t) result = >>> Config_error.unknown_options) in List.iter Error.print_config_error (errors @ unused_keys); - ok @@ Config.{url; assets; trees; foreign; home; theme} + ok + @@ Config.{url; assets; trees; foreign; home; theme; path = Some filename} let parse_forest_config_string str = let lexbuf = Lexing.from_string str in @@ -185,3 +186,27 @@ let parse_forest_config_file ~env filename = let result = Result.map_error List.singleton @@ parse lexbuf filename in Sys.chdir @@ Filename.dirname filename; Result.bind result (validate ~env) + +let find_forest_config ~env ~start_dir = + let toml_files_in dir = + try + Eio.Path.read_dir Eio.Path.(Eio.Stdenv.fs env / dir) + |> List.filter (fun name -> Filename.extension name = ".toml") + |> List.sort String.compare + with _ -> [] + in + let try_candidate name dir = + match parse_forest_config_file ~env (Filename.concat dir name) with + | Ok config -> Some config + | Error _ -> None + in + let rec go dir = + match + List.find_map (fun name -> try_candidate name dir) (toml_files_in dir) + with + | Some _ as found -> found + | None -> + let parent = Filename.dirname dir in + if parent = dir then None else go parent + in + go start_dir diff --git a/lib/compiler/Config_parser.mli b/lib/compiler/Config_parser.mli index f4fd8bf..3793964 100644 --- a/lib/compiler/Config_parser.mli +++ b/lib/compiler/Config_parser.mli @@ -12,3 +12,8 @@ val parse_forest_config_file : env:< fs : [> Eio.Fs.dir_ty] Eio.Path.t ; .. > -> string -> (Config.t, 'a Config_error.t list) result + +val find_forest_config : + env:< fs : [> Eio.Fs.dir_ty] Eio.Path.t ; .. > -> + start_dir:string -> + Config.t option diff --git a/lib/compiler/Driver.ml b/lib/compiler/Driver.ml index 0567554..ccbfe71 100644 --- a/lib/compiler/Driver.ml +++ b/lib/compiler/Driver.ml @@ -120,8 +120,7 @@ let batch_run ?(progress = Phases.no_progress) ~env ~(config : Config.t) ~dev () in go Load_configured_dirs forest -let language_server ~env ~config = - let init = State.make ~env ~config ~dev:true in +let load_configured_dirs (state : State.t) : State.t = let rec go action state = match action with | Quit Fail | Quit Finished -> state @@ -130,5 +129,8 @@ let language_server ~env ~config = 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 + let _ = update Plant_assets state in + go Load_configured_dirs state + +let language_server ~env ~config = + load_configured_dirs (State.make ~env ~config ~dev:true) diff --git a/lib/compiler/State.ml b/lib/compiler/State.ml index 8497795..29df978 100644 --- a/lib/compiler/State.ml +++ b/lib/compiler/State.ml @@ -18,7 +18,7 @@ type diagnostics = Diagnostics_store.t type t = { env: Eio_unix.Stdenv.base; dev: bool; - config: Config.t; + mutable config: Config.t; index: Tree.t URI.Tbl.t; duplicates: Duplicates.t; last_good_articles: T.content T.article URI.Tbl.t; diff --git a/lib/core/Config.ml b/lib/core/Config.ml index 6c22632..41e2d51 100644 --- a/lib/core/Config.ml +++ b/lib/core/Config.ml @@ -14,6 +14,7 @@ type t = { url: URI.t; home: URI.t; theme: string option; + path: string option; } [@@deriving show, repr] @@ -28,11 +29,13 @@ let default ?(url = default_url) () : t = url; home = URI.named_uri ~base:url "index"; theme = None; + path = None; } let make ?(trees = []) ?(assets = []) ?(foreign = []) ?(url = default_url) - ?(home = URI.of_string_exn "http://forest.local/index") ?(theme = None) () = - {trees; assets; foreign; url; home; theme} + ?(home = URI.of_string_exn "http://forest.local/index") ?(theme = None) + ?(path = None) () = + {trees; assets; foreign; url; home; theme; path} let home_uri config = config.home diff --git a/lib/core/Config.mli b/lib/core/Config.mli index 67ee7dc..ba56467 100644 --- a/lib/core/Config.mli +++ b/lib/core/Config.mli @@ -14,6 +14,7 @@ type t = { url: URI.t; home: URI.t; theme: string option; + path: string option; } [@@deriving show] @@ -24,6 +25,7 @@ val make : ?url:URI.t -> ?home:URI.t -> ?theme:string option -> + ?path:string option -> unit -> t diff --git a/lib/frontend/test/Test_config.ml b/lib/frontend/test/Test_config.ml index 88513ea..5cd257b 100644 --- a/lib/frontend/test/Test_config.ml +++ b/lib/frontend/test/Test_config.ml @@ -28,6 +28,7 @@ let test_parsing () = }; ]; theme = Some "theme"; + path = Some ""; }) begin Result.map_error (Fun.const ()) @@ -53,6 +54,7 @@ let test_missing_fields () = url = URI.of_string_exn "/"; home = URI.of_string_exn "/index/"; theme = None; + path = Some ""; } (Result.get_ok @@ Config_parser.parse_forest_config_string diff --git a/lib/language_server/Did_open.ml b/lib/language_server/Did_open.ml index 587d21c..057ce94 100644 --- a/lib/language_server/Did_open.ml +++ b/lib/language_server/Did_open.ml @@ -13,15 +13,36 @@ open struct module L = Lsp.Types end +let show_error message = + Publish.broadcast + @@ Lsp.Server_notification.ShowMessage + (L.ShowMessageParams.create ~message ~type_:L.MessageType.Error) + +let ensure_forest_loaded forest lsp_uri = + if forest.State.config.path <> None then () + else + let env = State.env forest in + let start_dir = Filename.dirname (Lsp.Uri.to_path lsp_uri) in + match Config_parser.find_forest_config ~env ~start_dir with + | None -> + show_error + (Printf.sprintf "No forest config found in %s or any parent directory." + start_dir) + | Some config -> + forest.State.config <- config; + ignore (Driver.load_configured_dirs forest); + Forester_frontend.Forester.install_default_write_hooks ~mode:Dynamic + forest; + ignore (Link_checker.check_all ~forest); + let addr_completions = Completion.populate_addr_completions ~forest in + Lsp_state.modify (fun s -> {s with addr_completions}) + let compute (params : L.DidOpenTextDocumentParams.t) = let lsp_uri = params.textDocument.uri in - (* let path = Lsp.Uri.to_path lsp_uri in *) let Lsp_state.{forest; _} = Lsp_state.get () in + ensure_forest_loaded forest lsp_uri; let document = Lsp.Text_document.make ~position_encoding:`UTF16 params in let uri = URI.of_lsp_uri ~base:forest.config.url lsp_uri in - (* let source : Tree.source = - `String {name = Some path; content = Lsp.Text_document.text document} - in *) forest.={uri} <- Tree (Tree.Loaded.create document); Lsp_state.modify (fun ({forest; _} as lsp_state) -> let new_forest = diff --git a/lib/language_server/Forester_lsp.ml b/lib/language_server/Forester_lsp.ml index 07af0ae..f619d1e 100644 --- a/lib/language_server/Forester_lsp.ml +++ b/lib/language_server/Forester_lsp.ml @@ -207,12 +207,28 @@ let rec event_loop () = if should_shutdown () then shutdown () else event_loop () | None -> Eio.traceln "Recieved an invalid message. Shutting down...@." -let start ~env ~port ~serve ~(config : Config.t) = +let start ~env ~port ~serve ~(config_path : string option) = let lsp_io = LspEio.init env in + let placeholder = Config.make () in + let config = + match config_path with + | Some path -> begin + match Config_parser.parse_forest_config_file ~env path with + | Ok config -> config + | Error errors -> + List.iter Error.print_config_error errors; + placeholder + end + | None -> placeholder + in (* FIXME: A "batch run" should fail early. The lsp should start even when there are errors *) - let forest = Driver.language_server ~env ~config in + let forest = + if config.path = None then State.make ~env ~config ~dev:true + else Driver.language_server ~env ~config + in Forester_frontend.Forester.install_default_write_hooks ~mode:Dynamic forest; + ignore (Link_checker.check_all ~forest); let addr_completions = Completion.populate_addr_completions ~forest in Eio.Switch.run @@ fun sw -> if serve then begin -- 2.51.2