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

code formatting, strict typechecking, linting, code cleanup and bugfixes #184

Merged
merged 20 commits into from
Apr 1, 2024

Conversation

Techatrix
Copy link
Collaborator

This PR includes some of the work I did on this extension that excludes any new major features I planned. I can split this into multiple PRs or remove some parts that are undesired if that helps with getting this reviewed and merged.

There are a lot of changes is in this PR. Here are some of the notable changes:

  • use prettier as the code formatter
  • enable typescript strict mode
  • enable recommended, strict and stylistic lints from typescript-eslint
  • remove the execCmd utility function and cleanup code around spawning child processes
  • fix an issue where ZLS wouldn't start or restart after changing zig.zls.path
  • consider the initial setup done when zig.path and zig.zls.path are set
  • show progress while downloading Zig or ZLS

Copy link
Member

@Vexu Vexu left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks!

src/extension.ts Outdated
activateZls(context)
});
export async function activate(context: vscode.ExtensionContext) {
await setupZig(context);
Copy link
Member

Choose a reason for hiding this comment

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

This previously used finally to activate the rest of the extension even if setupZig failed for some reason. Will that sill happen?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, I though there was a call to then instead of finally which is why I changed it. Let me correct that.

@Vexu
Copy link
Member

Vexu commented Apr 1, 2024

I wanted to link the release notes in the new Zig version notification so I pushed the change here as it was going to conflict.

Techatrix added 14 commits April 1, 2024 18:38
- add `typescript`
- add `typescript-eslint`
- make `@vscode/vsce` a dev dependency
- make `esbuild` a dev dependency
The legacy config system will be deprecated in ESLint v9.0.0
the progress message has also been update to not show "Zig" or "ZLS" twice
ast-check doesn't care about the current working directory
It is possible to ignore the initial setup and set the paths manually.
If that happens, no initial setup should happen.
The check that `zig.path` is set has been removed because ZLS is usable
without Zig.
Sending `null` instead of `"zig"` to ZLS will make it lookup in `$PATH`
instead of complaining that `"zig"` is not a valid file path.
just image you are writing Zig
This option has been removed in 47e6669
which I believe was accidental.
This will ensure that when handling responses from the user,
all possible responses are exhaustiveness handled.
@Vexu Vexu merged commit c1a293d into ziglang:master Apr 1, 2024
1 check passed
@Techatrix
Copy link
Collaborator Author

@Vexu I accidentally force pushed to this branch which removed the commits you have added. I was about to restore them but then your merged this PR a few seconds before I pushed again.

@Vexu
Copy link
Member

Vexu commented Apr 1, 2024

No worries, I pushed them directly to master so that I could release a new version.

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