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

make semantic token configuration an enum #1064

Merged
merged 1 commit into from
Mar 23, 2023

Conversation

Vexu
Copy link
Contributor

@Vexu Vexu commented Mar 14, 2023

Implements #338

I'm not sure how the config gen stuff works so any help on that would be appreciated.

Vscode companion pr ziglang/vscode-zig#90

@Techatrix
Copy link
Member

I'm not sure how the config gen stuff works so any help on that would be appreciated.

You can use config_gen by only editing src/config_gen/config.json and then running zig build gen which should update the readme, schema.json and Config.zig.
You can also run zig build gen -- --vscode-config-path /some/output/path.json which will also generate the json configuration options that you can then copy into package.json in vscode-zig.

I've never added support for enums in config_gen so until I take care of that, it can't be used. I will do a follow up PR to fix that.

And also, is it intentional that the default value is different between the ZLS config and the vscode-zig config?

schema.json Outdated Show resolved Hide resolved
@Vexu
Copy link
Contributor Author

Vexu commented Mar 14, 2023

And also, is it intentional that the default value is different between the ZLS config and the vscode-zig config?

Looks like I forgot to edit the default value for the zls config, it should probably be "full" as you suggested. The vscode extension already provides grammar based highlighting so for it the default should be partial.

@Techatrix
Copy link
Member

This patch with #1072 should update allow files accordingly after running zig build gen

diff --git a/src/config_gen/config.json b/src/config_gen/config.json
index 1ca86d9..3ac4b16 100644
--- a/src/config_gen/config.json
+++ b/src/config_gen/config.json
@@ -25,10 +25,15 @@
             "default": "true"
         },
         {
-            "name": "enable_semantic_tokens",
-            "description": "Enables semantic token support when the client also supports it",
-            "type": "bool",
-            "default": "true"
+            "name": "semantic_tokens",
+            "description": "Set level of semantic tokens. Partial only includes information that requires semantic analysis.",
+            "type": "enum",
+            "enum": [
+                "none",
+                "partial",
+                "full"
+            ],
+            "default": "full"
         },
         {
             "name": "enable_inlay_hints",

@Vexu Vexu force-pushed the semantic-tokens branch 2 times, most recently from 0af5ed6 to 4725008 Compare March 20, 2023 11:58
@Vexu Vexu requested a review from Techatrix March 20, 2023 22:06
Copy link
Member

@Techatrix Techatrix left a comment

Choose a reason for hiding this comment

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

👍
If you didn't know, tres can be used to avoid having to do the jsonStringify manually.

diff --git a/src/main.zig b/src/main.zig
index 19cc527..b3e6251 100644
--- a/src/main.zig
+++ b/src/main.zig
@@ -3,6 +3,7 @@ const zig_builtin = @import("builtin");
 const build_options = @import("build_options");
 const tracy = @import("tracy.zig");
 const known_folders = @import("known-folders");
+const tres = @import("tres");
 const Config = @import("Config.zig");
 const configuration = @import("configuration.zig");
 const Server = @import("Server.zig");
@@ -129,7 +130,7 @@ fn updateConfig(
         var buffer = std.ArrayListUnmanaged(u8){};
         defer buffer.deinit(allocator);
 
-        try std.json.stringify(cfg, .{}, buffer.writer(allocator));
+        try tres.stringify(cfg, .{}, buffer.writer(allocator));
         const header = Header{ .content_length = buffer.items.len };
         try header.write(false, file.writer());
         try file.writeAll(buffer.items);

Feels weird to review your code, instead of the other ways round...

@Vexu
Copy link
Contributor Author

Vexu commented Mar 20, 2023

If you didn't know, tres can be used to avoid having to do the jsonStringify manually.

I did not know, it's a bit nicer so thanks for the suggestion.

Feels weird to review your code, instead of the out ways round...

Nice to swap roles occasionally.

@Vexu
Copy link
Contributor Author

Vexu commented Mar 22, 2023

Does this need anything else?

@leecannon leecannon merged commit 61b42ca into zigtools:master Mar 23, 2023
@Vexu Vexu deleted the semantic-tokens branch March 23, 2023 15:37
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.

3 participants