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

feat: add zls support #58

Merged
merged 26 commits into from
Aug 19, 2024
Merged

Conversation

jinzhongjia
Copy link
Contributor

@jinzhongjia jinzhongjia commented Jul 31, 2024

Features

  • Code logic organization and optimization
  • Add new command remove
  • Destroyed the original download prompt message

Since the original download information is destroyed, it is not clear what kind of interaction should be better.

src/tools.zig Outdated Show resolved Hide resolved
@jinzhongjia jinzhongjia marked this pull request as ready for review August 7, 2024 14:18
@jinzhongjia
Copy link
Contributor Author

Sorry, I made a lot of changes to the source code, but I think the changes were worth it and the readability should be improved by now

Copy link
Owner

@hendriknielaender hendriknielaender left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great effort on the recent updates! The improvements are appreciated. It's exciting to see that we'll soon support zls as well, which will significantly enhance zvm. Keep up the excellent work!

src/install.zig Outdated Show resolved Hide resolved
src/config.zig Outdated Show resolved Hide resolved
src/config.zig Show resolved Hide resolved
src/config.zig Outdated Show resolved Hide resolved
src/tools.zig Outdated Show resolved Hide resolved
src/command.zig Outdated Show resolved Hide resolved
src/command.zig Outdated Show resolved Hide resolved
src/alias.zig Outdated Show resolved Hide resolved
src/alias.zig Outdated Show resolved Hide resolved
src/alias.zig Outdated Show resolved Hide resolved
@jinzhongjia
Copy link
Contributor Author

jinzhongjia commented Aug 8, 2024

Yes, actually I recommend not using debug.print, because it will write directly to stdout
Maybe we can override the default log function. The zig standard library provides a better overwriting method.
tools.zig has been split into source files under util

@jinzhongjia
Copy link
Contributor Author

There is no problem when testing under Linux. I have not tested on Windows.

@hendriknielaender
Copy link
Owner

There is no problem when testing under Linux. I have not tested on Windows.

Alright, I will test it for Windows and macOS

@jinzhongjia
Copy link
Contributor Author

If the tests are OK, I think it can be merged, but it should not be released immediately because the interaction is broken.

I will try to add local compilation support for zzls and master update support for zig and zls later

src/command.zig Outdated Show resolved Hide resolved
src/command.zig Outdated Show resolved Hide resolved
src/command.zig Outdated Show resolved Hide resolved
src/command.zig Outdated Show resolved Hide resolved
@jinzhongjia
Copy link
Contributor Author

Hey man, I don't know if you are willing to accept this PR now, If you are not willing to accept the PR, you can close it

@hendriknielaender
Copy link
Owner

Hey man, I don't know if you are willing to accept this PR now, If you are not willing to accept the PR, you can close it

I'm willing to accept this, but the minimum requirement for me would be that this is tested on windows as well. Last week I didn't had time to verify your changes, feel free to manual test this.

@jinzhongjia
Copy link
Contributor Author

now, windows test ok

Due to the introduction of new code, macos and linux platforms need to be retested

@jinzhongjia
Copy link
Contributor Author

I will try linux testing later

@jinzhongjia
Copy link
Contributor Author

Linux tests OK.

@hendriknielaender hendriknielaender merged commit 4354f07 into hendriknielaender:main Aug 19, 2024
4 checks passed
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.

2 participants