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.

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

[WIP] Local LSP server that extends and enhances remote LSP server #119

Closed
wants to merge 1 commit into from

Conversation

SamuelBrucksch
Copy link
Contributor

WIP, heavy modification of folder structure was required, so there might be more changes in the files changed section than actual changes.

This will fix #95 when all the work is done.

Signed-off-by: Samuel Brucksch sasliga@freenet.de (github: SamuelBrucksch)

@SamuelBrucksch SamuelBrucksch force-pushed the lsp-server branch 3 times, most recently from ef11137 to d718e0c Compare December 9, 2018 10:05
@Confectrician
Copy link
Collaborator

heavy modification of folder structure was required

Can you explain that a bit?

ALso i am not sure about thos many force pushes here.
Why do you need to force push al that stuff?
What is the error, when you try to push without force?
This is a bit confusing.

@Confectrician
Copy link
Collaborator

Next question:
Is this now only an lsp server pr or did you do code refactoring/prettier stuuf on many places too?

Signed-off-by: Samuel Brucksch <sasliga@freenet.de> (github: SamuelBrucksch)
@MHerbst
Copy link

MHerbst commented Dec 9, 2018

After a very rough scan it seems to me that there are some overlapping changes with #117. This would explain the "force push".

@Confectrician
Copy link
Collaborator

I will merge this one really fast.

Maybe you should wait for that and do a rebase then @SamuelBrucksch

@SamuelBrucksch
Copy link
Contributor Author

SamuelBrucksch commented Dec 9, 2018

Hi,

the LSP somehow did not work well when it was in the same module. So i seperated it like in the lsp example here:
https://github.com/Microsoft/vscode-extension-samples/tree/master/lsp-sample

Now we have a client folder that includes the extension and a server folder that includes the LSP server. This has also the benefit, that we can deploy the LSP server somewhere else if we want.

The force pushes were because i missed some things and i did not want unnecessary git commit messages to show up. However if that is not wished and instead many commit messages do not matter i can also do single commits. Thats fine as well for me.

The changes from #117 are not reflected here yet, however we somehow have to take a close look when merging together as we will most likely get some merge conflicts because of folder structure changes.

This is only a LSP server push, however because of my settings in vscode some of the files appear to be modified but its actually just formatting. This is why i proposed to use prettier and/or standard to reduce unrelated formatting changes to a minimum. If there is no clear instruction on how to setup vscode formatting, there is now way around this.

But i can also do a clean fresh PR when you merged your changes, then i can also revert all those formatting changes and its easier to see what actually changed.

@Confectrician
Copy link
Collaborator

However if that is not wished and instead many commit messages do not matter i can also do single commits.

Please. I love many commits. ❤️
You can follow up the development and maybe learn something on the way.
We are doing squash and merge anyway here so it doesn't matter for the "lategame".

I don't know why, but anytime i read a "force" within git my alarm bells ring. 😄
I just have a bad feeling then.

Code formatting

You are fully right that there needs to be something like a standard.

It didn't matter, while only kuba worked here and i did some json snippets.
This is different with more contributors and we have to adress it.

But since this is not a standalone project i would like to have some opinion from @kubawolanin or @kaikreuzer too.
We definetely have coding standards for bindings and framework contributions.
So maybe we can also use (at least parts) of them to get a coverage with other parts of openHAB and keep a organisation wide clear structure.

I know that this answer may not satisfy you know, but i think this is nothing we have to decide in the next minutes.
If we agree on something, we have to refactor some parts anyway after decision.

@Confectrician
Copy link
Collaborator

Maybe you could share your settings here and maybe we can already add something to the .vscode folder.
I think there is already a sttings.json that takes care of tab size and other stuff.

@SamuelBrucksch
Copy link
Contributor Author

Will close this and reopen another one that is cleaner. Will disable save on format so i do not accidentaly push only formatted files.

@SamuelBrucksch SamuelBrucksch deleted the lsp-server branch December 9, 2018 12:34
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.

Autocompletion/syntax checking extremely slow/unusable
3 participants