Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(ocamllsp): workspace symbols fallback to _build folder #1097

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+20 −5
Open

fix(ocamllsp): workspace symbols fallback to _build folder #1097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ce21883
test(ocamllsp): add workspace symbol test to showcase issue with gene…
tatchi May 10, 2023
63645a4
fix(ocamllsp): workspace symbols fallback to _build folder if source …
tatchi May 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions ocaml-lsp-server/src/workspace_symbol.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,8 @@ end

exception Cancelled

let symbols_from_cm_file ~filter root_uri (cancel : Fiber.Cancel.t option)
cm_file =
let symbols_from_cm_file ~build_dir ~filter root_uri
(cancel : Fiber.Cancel.t option) cm_file =
let cmt =
let filename = string_of_cm cm_file in
let cancelled =
Expand All @@ -230,7 +230,14 @@ let symbols_from_cm_file ~filter root_uri (cancel : Fiber.Cancel.t option)
in
let loc = Mbrowse.node_loc browse in
let fname = loc.loc_start.pos_fname in
let uri = Uri.of_path (Filename.concat root_uri fname) in
let uri =
let sourcefile = Filename.concat root_uri fname in
let path =
if Fpath.exists (Fpath.of_string sourcefile) then sourcefile
else build_dir ^ "/" ^ fname
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably also verify that the uri in _build folder exist? And if not remove the entry ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better if we use Fpath for path concatenation, instead of doing it by hand

in
Uri.of_path path
in
filter (Document_symbol.symbols_of_outline uri outline))
| _ -> [])

Expand Down Expand Up @@ -288,7 +295,7 @@ let run ({ query; _ } : WorkspaceSymbolParams.t)
Uri.to_path uri
in
List.concat_map
~f:(symbols_from_cm_file ~filter path cancel)
~f:(symbols_from_cm_file ~build_dir ~filter path cancel)
cm_files))
with Cancelled -> Error `Cancelled

Expand Down
2 changes: 2 additions & 0 deletions ocaml-lsp-server/test/e2e/__tests__/workspace-symbol.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ describe("workspace/symbol", () => {
"lib_x 12 /workspace_symbol_A/lib/lib.ml 6:0 6:14",
"user 15 /workspace_symbol_A/lib/lib.ml 3:0 5:1",
"name 7 /workspace_symbol_A/lib/lib.ml 4:2 4:14",
"hello 12 /workspace_symbol_A/_build/default/lib/gen.ml 0:0 0:19",
"t 15 /workspace_symbol_A/lib/LibTypes.mli 0:0 0:15",
"x 12 /workspace_symbol_A/vendor/vendored_lib.ml 0:0 0:9",
]
Expand Down Expand Up @@ -200,6 +201,7 @@ describe("workspace/symbol", () => {
"lib_x 12 /workspace_symbol_A/lib/lib.ml 6:0 6:14",
"user 15 /workspace_symbol_A/lib/lib.ml 3:0 5:1",
"name 7 /workspace_symbol_A/lib/lib.ml 4:2 4:14",
"hello 12 /workspace_symbol_A/_build/default/lib/gen.ml 0:0 0:19",
"t 15 /workspace_symbol_A/lib/LibTypes.mli 0:0 0:15",
"x 12 /workspace_symbol_A/vendor/vendored_lib.ml 0:0 0:9",
"workspace_B 12 /workspace_symbol_B/main.ml 0:0 0:31",
Expand Down
8 changes: 7 additions & 1 deletion ocaml-lsp-server/test/e2e/__tests__/workspace_symbol_A/lib/dune
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,11 @@
(flags :standard -w -32-38-27-34)
(name lib))


(copy_files# ../vendor/*.ml{,i})

(rule
(target gen.ml)
(action
(with-stdout-to
%{target}
(echo "let hello = \"world\""))))