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

Allow lang server restart #85

Merged
merged 11 commits into from
Feb 9, 2021

Conversation

clayreimann
Copy link
Collaborator

A bunch of refactoring as well as adding commands for swift clean, swift run, and restarting the language server.

@clayreimann clayreimann marked this pull request as draft November 26, 2020 02:43
@vknabel
Copy link
Owner

vknabel commented Dec 5, 2020

Great changes so far!
I had quite a lot to do the last weeks. Should be better this month. :)

@clayreimann
Copy link
Collaborator Author

@vknabel A lot of progress here.

  • The language server can be restarted (I've only confirmed sourcekitd because that's what I use)
  • Added commands to run, stop, clean
  • Cleaned up (IMO) the code for better understanding and maintenance

Could be merged as-is, but I think the configuration still needs to be cleaned up a bit. (It should all use the same prefix sde.SECTION.setting and I'd like to export an object from the configuration helper that mirrors the options–but allows us to hide the specific configuration names so we can migrate from what is to the new strings.)

@clayreimann clayreimann marked this pull request as ready for review December 27, 2020 19:23
@clayreimann
Copy link
Collaborator Author

Actually @vknabel I think it would make sense to merge this without the settings cleanup so we can better pinpoint where any bugs come from.

This is already a pretty major rewrite and changing how we fetch config any more seems like asking for more headaches

@vknabel
Copy link
Owner

vknabel commented Jan 14, 2021

Hey @clayreimann. Sorry for the late response and a huge thank you for all your amazing work!

I have already started reviewing this PR. I will try to test everything as good as possible (on macOS and Linux), which is quite time consuming, but it will be worth it.

Once I find bugs, I will probably fix them directly, if that's okay for you.

Currently I am not sure how we handle the release and renamed configs or commands. Technically it would be a breaking change and would require a major version bump.
But this should be fine.
Might be a great opportunity to delete unused features.

@vknabel vknabel self-assigned this Feb 9, 2021
@vknabel vknabel merged commit 6d1c41c into vknabel:master Feb 9, 2021
vknabel added a commit that referenced this pull request Feb 9, 2021
@vknabel
Copy link
Owner

vknabel commented Feb 9, 2021

Thanks a lot @clayreimann!
I know it took quite a while. This PR really helps in the long term!
In b247437 I made a few fixes, added some changelogs and mentioned you in the README.

@vknabel
Copy link
Owner

vknabel commented Feb 9, 2021

I created a beta release tag: 2.11.0-beta.0.

I try to test it bit, but I believe it should work fine. Will release it soon.

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.

2 participants