-
Notifications
You must be signed in to change notification settings - Fork 81
Enable mlx in documentSelector and a few small features #1964
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.
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
Enable mlx in documentSelector and a few small features #1964
Conversation
…into include-mlx-documentSelector * 'master' of github.com:/ocamllabs/vscode-ocaml-platform: (36 commits) Bump esbuild from 0.25.11 to 0.25.12 (ocamllabs#1996) Lock file maintenance flake.lock: Update (ocamllabs#1995) Mise Bump tar from 7.5.1 to 7.5.2 (ocamllabs#1994) Bump @tsconfig/strictest from 2.0.6 to 2.0.7 (ocamllabs#1993) Bump @types/node from 24.9.1 to 24.9.2 (ocamllabs#1991) Bump @biomejs/biome from 2.3.1 to 2.3.2 (ocamllabs#1992) Bump @biomejs/biome from 2.3.0 to 2.3.1 (ocamllabs#1989) Bump actions/upload-artifact from 4 to 5 (ocamllabs#1988) Bump actions/download-artifact from 5 to 6 (ocamllabs#1987) Update opam files Prepare 1.32.4 (ocamllabs#1986) Lock file maintenance Fix keybinding when conditions and unsupported language filters (ocamllabs#1985) Remove `ocaml.ocamllex` from type-dependent command conditions (ocamllabs#1984) Fix editor focus condition for interface keybindings (ocamllabs#1983) flake.lock: Update (ocamllabs#1982) Bump ocamlformat from 0.27.0 to 0.28.1 (ocamllabs#1981) Bump @biomejs/biome from 2.2.7 to 2.3.0 (ocamllabs#1980) ...
|
Updated the PR and resolved the conflicts with #1985. I endup with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
src/ast_editor.ml:87
- The
get_pp_pathfunction handles.mlxfiles asStructureOcaml(line 71) but the preprocessing path generation at line 87 only strips.mlsuffix. This will fail for.mlxfiles sinceString.chop_suffix_exnwill raise an exception when the file doesn't have a.mlextension. Consider adding a case to handle.mlx` files or updating the pattern match to handle both extensions.
| Structure `Ocaml ->
Some (String.chop_suffix_exn ~suffix:".ml" relative ^ ".pp.ml")
src/switch_impl_intf.ml:12
- The comment on line 12 states 'If the source file was a .ml or .re' but the code now also supports .mlx files (line 8). Update the comment to include .mlx files.
(* If the source file was a .ml or .re, infer the interface *)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| "command": "ocaml.search-by-type", | ||
| "when": "editorLangId == ocaml || editorLangId == ocaml.interface || editorLangId == reason" | ||
| "when": "editorLangId == ocaml || editorLangId == ocaml.interface || editorLangId == reason || editorLangId == ocaml.mlx" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we could probably add editorTextFocus for correctness
This PR enables ocaml.mlx in a few places:
I leaves a few todos: