-
Notifications
You must be signed in to change notification settings - Fork 84
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
bs-platform 5.1.0 incompatibility breaks reason-vscode or lsp #320
Comments
Tried to remove node modules and reinstall everything with:
or tried to remove .node_modules/.lsp but neither had an effect |
This line may be relevant, I wonder what kind of internals does RLS rely on Buckle so that we can add some tests on the CI
|
hmmm is bs-platform 5.1.0 no longer on ocaml 4.02? |
git>npm i -g [email protected] >/dev/null
git>bsc -v
BuckleScript 5.1.0 (Using OCaml4.02.3+BS )
… On Aug 20, 2019, at 3:13 AM, Jared Forsyth ***@***.***> wrote:
hmmm is bs-platform 5.1.0 no longer on ocaml 4.02?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#320?email_source=notifications&email_token=AAFWMKYKG2GEQFBZDKFWXSTQFLWETA5CNFSM4IMOPH3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4UABSQ#issuecomment-522715338>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFWMK6LL73NLOYKGQQEXQLQFLWETANCNFSM4IMOPH3A>.
|
Would any one have a look why it is broken? Thanks
… On Aug 20, 2019, at 5:47 PM, Hongbo Zhang ***@***.***> wrote:
git>npm i -g ***@***.*** >/dev/null
git>bsc -v
BuckleScript 5.1.0 (Using OCaml4.02.3+BS )
> On Aug 20, 2019, at 3:13 AM, Jared Forsyth ***@***.*** ***@***.***>> wrote:
>
> hmmm is bs-platform 5.1.0 no longer on ocaml 4.02?
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub <#320?email_source=notifications&email_token=AAFWMKYKG2GEQFBZDKFWXSTQFLWETA5CNFSM4IMOPH3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4UABSQ#issuecomment-522715338>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFWMK6LL73NLOYKGQQEXQLQFLWETANCNFSM4IMOPH3A>.
>
|
@jaredly how can i help ? |
In my case I get this error : |
for a repro you can check this: sync/reason-graphql-demo@e207233 |
Ok, I've pushed a branch w/ a repro in reason-language-server, along with some extra debug output. https://github.com/jaredly/reason-language-server/tree/diagnose-5.1.0 This demonstrates first 5.0.6 working, and then 5.1.0 failing. Here's the invocation that
it exits w/ error code 2, and the following stderr
However, all of the referenced paths in that command exist. |
looking into it |
@jaredly how do you assemble the command line flags,
|
Here is the log I got, it seems we need have a better way to support rls, let me know what kind of internals API rls needs
|
@bobzhang What do you mean " |
I just checked |
ermmm |
Indeed, bsc used to be an internal command that we recently made it useful for single file toying around (https://bucklescript.github.io/blog/2019/08/12/release-5-10-0) |
So the place where I need to use bsc directly is for "as you type" analysis -- I need to get type information without updating the file in the file system where bsb expects it. |
Would you give me a detail what you want to achieve, we can provide even better integration in most cases. From what I can tell, rls try to assemble command line args from .merlin which could be less consistent. The thing is that the merlin specification is not complete and does not understand the whole project structure, bsb understand the whole project and can provide all you need in a consistent way (consistent with the real build flags)
bsc was indeed internal stuff, until in this release, we make it public so that people can toy around without setting up a project |
hi @jaredly let me know what I can help here. |
Latest version breaks the reason-language-server. Revert when jaredly/reason-language-server#320 is fixed.
I checked this was probably due to the change of |
Thanks for looking into it 😁 As far as I can tell, the key difference between RLS and BSB is that RLS provides feedback before the file is saved (a feature I believe merlin also supports?). For that to work it’s saving a temporary file and running bsc directly. It might be difficult to replicate that with BSB. |
@jaredly it seems
it may be that refmt does not follow the protocol closely |
I made a tentative fix, would anyone confirm it would be fixed in this PR rescript-lang/rescript#3823 |
@bobzhang can confirm this fixes it, amazing thank you |
@sync working on this, give me a sec |
@sync pushed a new commit, can you confirm it is good now |
trying this now |
All good can confirm it is working now thank you so much !! Tested in vscode and vim (with coc and lsp) |
@bobzhang the feature at stake is as-you-type compilation, which means the |
I'm seeing an issue using ReasonML code:
RLS error, highlighting the word
It also shows |
Not seeing this issue on 5.2.0. |
6.1.0 is paired with 5.1.0 we will release 6.2.0 soon
发自我的iPhone
…------------------ Original ------------------
From: Bikal Lem <[email protected]>
Date: Tue,Sep 24,2019 7:27 PM
To: jaredly/reason-language-server <[email protected]>
Cc: Hongbo Zhang <[email protected]>, Mention <[email protected]>
Subject: Re: [jaredly/reason-language-server] bs-platform 5.1.0 incompatibility breaks reason-vscode or lsp (#320)
Seeing the same issue on [email protected]
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
When upgrading to bs-platform 5.1.0 on a ReasonReact project for example when opening this project using vscode:
https://github.com/sync/reason-graphql-demo
I get the follow error:
See attached debug log
debug.log
The text was updated successfully, but these errors were encountered: