Skip to content

Conversation

@nojaf
Copy link
Member

@nojaf nojaf commented Nov 19, 2025

Fixes the hover in rescript-lang/rescript-vscode#1144

@zth can you please review, this is what the machine came up with.
It worked on my machine, but I don't know this part of the codebase.

@nojaf
Copy link
Member Author

nojaf commented Nov 19, 2025

@mediremi feel free to take a look as well.
You might know this area as well.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 19, 2025

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@8023

@rescript/darwin-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-arm64@8023

@rescript/darwin-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-x64@8023

@rescript/linux-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-arm64@8023

@rescript/linux-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-x64@8023

@rescript/runtime

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/runtime@8023

@rescript/win32-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/win32-x64@8023

commit: 4179647

Copy link
Member

@mediremi mediremi left a comment

Choose a reason for hiding this comment

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

The added hover test is not affected by the changes you made in analysis/src/Hover.ml. If you revert your changes in analysis/src/Hover.ml and rerun make test-analysis, you'll see that tests/analysis_tests/tests/src/expected/Hover.res.txt is unchanged.

@nojaf
Copy link
Member Author

nojaf commented Nov 20, 2025

Hmm, I don't get it. The same hover command with my local compiler versus the analysis binary in works for me:

(base) nojaf@nojaf-mbp test-stdlib % /Users/nojaf/Projects/test-stdlib/node_modules/.pnpm/@rescript+darwin-arm64@12.0.0-rc.4/node_modules/@rescript/darwin-arm64/bin/rescript-editor-analysis.exe debug-dump verbose hover /Users/nojaf/Projects/test-stdlib/src/App.res 11 19 /var/folders/mt/_svhkyhd5cjfcbg6zf6nyzf40000gn/T/rescript_format_file_66362_4 true
null

(base) nojaf@nojaf-mbp test-stdlib % /Users/nojaf/Projects/rescript/_build/default/analysis/bin/main.exe debug-dump verbose hover /Users/nojaf/Projects/test-stdlib/src/App.res 11 19 /var/folders/mt/_svhkyhd5cjfcbg6zf6nyzf40000gn/T/rescript_format_file_66362_4 true
{"contents": {"kind": "markdown", "value": "```rescript\nmodule Stdlib_BigInt: {\n  type t = bigint\n  let asIntN: (~width: int, bigint) => bigint\n  let asUintN: (~width: int, bigint) => bigint\n  let fromStringOrThrow: string => bigint\n  let fromString: string => option<bigint>\n  let fromStringExn: string => bigint\n  let fromInt: int => bigint\n  let fromFloatOrThrow: float => bigint\n  let fromFloat: float => option<bigint>\n  let toString: (bigint, ~radix: int=?) => string\n  let toStringWithRadix: (bigint, ~radix: int) => string\n  let toLocaleString: bigint => string\n  let toFloat: bigint => float\n  let toInt: bigint => int\n  let add: (bigint, bigint) => bigint\n  let sub: (bigint, bigint) => bigint\n  let mul: (bigint, bigint) => bigint\n  let div: (bigint, bigint) => bigint\n  let mod: (bigint, bigint) => bigint\n  let bitwiseAnd: (bigint, bigint) => bigint\n  let bitwiseOr: (bigint, bigint) => bigint\n  let bitwiseXor: (bigint, bigint) => bigint\n  let bitwiseNot: bigint => bigint\n  let shiftLeft: (bigint, bigint) => bigint\n  let shiftRight: (bigint, bigint) => bigint\n  let ignore: bigint => unit\n  let land: (bigint, bigint) => bigint\n  let lor: (bigint, bigint) => bigint\n  let lxor: (bigint, bigint) => bigint\n  let lnot: bigint => bigint\n  let lsl: (bigint, bigint) => bigint\n  let asr: (bigint, bigint) => bigint\n}\n```"}}

I don't get why the release version just outputs null.

let ( /+ ) = Filename.concat

(*
Here is the real problem:
Copy link
Member Author

Choose a reason for hiding this comment

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

@zth I would use an environment variable here to avoid breaking all the analysis commands.
Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

I think that's fine, we use env variables for other stuff as well. So go for it!

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, ready for review.

@nojaf nojaf changed the title Fix hover for Stdlib module Analysis: resolve @rescript/runtime via environment variable RESCRIPT_RUNTIME Nov 20, 2025
@nojaf nojaf marked this pull request as ready for review November 20, 2025 07:57
@nojaf
Copy link
Member Author

nojaf commented Nov 20, 2025

Can't easily add a test for this here.
Worked on my machine.

@nojaf nojaf requested a review from mediremi November 20, 2025 10:30
@nojaf nojaf enabled auto-merge (squash) November 20, 2025 10:30
@nojaf nojaf dismissed mediremi’s stale review November 20, 2025 10:31

No longer applicable

@nojaf nojaf merged commit fa64b08 into rescript-lang:master Nov 20, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants