Skip to content
This repository has been archived by the owner on Jan 29, 2024. It is now read-only.

fix @hegel/language-server #260

Merged
merged 9 commits into from
Jun 16, 2020
Merged

Conversation

thecotne
Copy link
Contributor

@thecotne thecotne commented May 25, 2020

i am trying to make this one workspace for vscode so you can compile and test everything at same time


this needs to be updated after #257 and #258 is merged

also language-server does not work for reasons beyond my comprehension :D


currently debug mode works and vsix version does not

@Kapelianovych
Copy link
Contributor

So with yarn inside project we should use only local packages?

@thecotne
Copy link
Contributor Author

@YevhenKap

yes we use local packages but only use files from other packages that are published on npm

i want to even limit usage to only main entry of package so that file structure inside package is private and only main entry is used by other packages

@thecotne thecotne marked this pull request as ready for review May 27, 2020 23:23
@thecotne
Copy link
Contributor Author

everything seems fine but i can't get it working : /

this is what i got

Screen Shot 2020-05-28 at 03 31 13

and that's it no diagnostic no autocomplete no tooltips

@Kapelianovych
Copy link
Contributor

@thecotne move hegel/packages/language-server/.vscode/extensions.json to hegel/.vscode/extensions.json (Its not need to be alone :) ).
Also set proper paths to typings:
Bildschirmfoto 2020-05-30 um 10 01 41

@Kapelianovych
Copy link
Contributor

Kapelianovych commented May 30, 2020

And probably we should remove "@hegel/cli": "^0.0.43", "@hegel/core": "^0.0.43", "@hegel/typings": "^0.0.41" from language-server/package.json as it is meant to be get from npm.

@JSMonk
Copy link
Owner

JSMonk commented May 30, 2020

Extension starts successfully, but on code below Hegel throw strange error.

function add(f: number, s: number): number {
  return f + s;
}

add(5, '');
Bildschirmfoto 2020-05-30 um 09 55 04

This is internal error thrown by Hegel, so it is throws away by extension as improper diagnostic error. Core is failed, so whole extension is down (actually it is up, but does not receive diagnostic information and all info about types, errors, variables is undefined).

Will try ti fix it soon.

@thecotne
Copy link
Contributor Author

with latest changes if i open extension in debug mode it works but packaged version still does not work (i think it's an issue with dependencies)

@YevhenKap

Extension starts successfully, but on code below Hegel throw strange error.

try latest version in debug mode (make sure to install dependencies and rebuild both cli and core)

move hegel/packages/language-server/.vscode/extensions.json to hegel/.vscode/extensions.json (Its not need to be alone :) ).

it is moved see https://github.com/JSMonk/hegel/pull/260/files#diff-24c73412592dac9d8b9a739107c2c905

Also set proper paths to typings:

this was the problem after updating paths it started working in debug mode

And probably we should remove "@hegel/cli": "^0.0.43", "@hegel/core": "^0.0.43", "@hegel/typings": "^0.0.41" from language-server/package.json as it is meant to be get from npm.

all packages must explicitly list there dependencies because this is monorepo not monolith

packages are still separate it's just they are developed and tested together to ensure compatibility and to simplify implementation of features that need changes in multiple packages

@thecotne
Copy link
Contributor Author

thecotne commented Jun 2, 2020

@JSMonk i expected VS Code extension to only contain language server client and language server itself to be part of cli like it is with flow (not sure about typescript)

so language server and cli in project is same version even if you have alpha/beta release of cli or old version of cli


@JSMonk also wanted to ask are you ok with removing typescript from @hegel/language-server maybe even migrating to hegel

@thecotne thecotne marked this pull request as draft June 2, 2020 15:23
@Kapelianovych
Copy link
Contributor

@JSMonk i expected VS Code extension to only contain language server client and language server itself to be part of cli like it is with flow (not sure about typescript)

so language server and cli in project is same version even if you have alpha/beta release of cli or old version of cli

Language server requires some values from vscode package. If we move LSP to cli, extensions for other editors will not be able to implemented. In this case extension must be completely rewritten.

@Kapelianovych
Copy link
Contributor

Kapelianovych commented Jun 4, 2020

And at current time we cannot package extension by vsce. Because vsce tool does not package dependencies neither by yarn, not by npm. See issue.

@thecotne thecotne marked this pull request as ready for review June 5, 2020 21:31
@thecotne
Copy link
Contributor Author

thecotne commented Jun 5, 2020

at this point i think it's in working condition and ready to be reviewed/merged...

@thecotne
Copy link
Contributor Author

thecotne commented Jun 5, 2020

for some reason i get this error

Screen Shot 2020-06-06 at 01 51 23

there is no index.js.git in my workspace and it only shows that error in editor when i click on that error if i just open index.js everything is ok

i am testing this on @hegel/cli

@JSMonk
Copy link
Owner

JSMonk commented Jun 15, 2020

@YevhenKap, will merge it after your approval.

@Kapelianovych
Copy link
Contributor

I think it can be merged. Maybe @thecotne should resolve conflict?

@thecotne
Copy link
Contributor Author

@YevhenKap resolved

@Kapelianovych
Copy link
Contributor

@thecotne you are cool man)

@JSMonk JSMonk merged commit 42b6084 into JSMonk:master Jun 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants