openhab / openhab-vscode

VS Code extension for openHAB configuration files
https://marketplace.visualstudio.com/items?itemName=openhab.openhab
Eclipse Public License 2.0
159 stars 47 forks source link

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

Closed SamuelBrucksch closed 5 years ago

SamuelBrucksch commented 5 years ago

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)

Confectrician commented 5 years ago

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 commented 5 years ago

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

MHerbst commented 5 years ago

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

Confectrician commented 5 years ago

I will merge this one really fast.

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

SamuelBrucksch commented 5 years ago

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 commented 5 years ago

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. :heart: 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. :smile: 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 commented 5 years ago

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 commented 5 years ago

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