better build-on-save

schema.json

@@ -15,14 +15,17 @@
             "default": true
         },
         "enable_build_on_save": {
-            "description": "Whether to enable build-on-save diagnostics",
+            "description": "Whether to enable build-on-save diagnostics. Will be automatically enabled if the `build.zig` has declared a 'check' step.",
             "type": "boolean",
-            "default": false
+            "default": null
         },
-        "build_on_save_step": {
-            "description": "Select which step should be executed on build-on-save",
-            "type": "string",
-            "default": "install"
+        "build_on_save_args": {
+            "description": "Specify which arguments should be passed to Zig when running build-on-save.\n\nIf the `build.zig` has declared a 'check' step, it will be preferred over the default 'install' step.",
+            "type": "array",
+            "items": {
+                "type": "string"
+            },
+            "default": []
         },
         "enable_autofix": {
             "description": "Whether to automatically fix errors on save. Currently supports adding and removing discards.",

src/Config.zig

@@ -10,11 +10,13 @@ enable_snippets: bool = true,
 /// Whether to enable function argument placeholder completions
 enable_argument_placeholders: bool = true,
 
-/// Whether to enable build-on-save diagnostics
-enable_build_on_save: bool = false,
+/// Whether to enable build-on-save diagnostics. Will be automatically enabled if the `build.zig` has declared a 'check' step.
+enable_build_on_save: ?bool = null,
 
-/// Select which step should be executed on build-on-save
-build_on_save_step: []const u8 = "install",
+/// Specify which arguments should be passed to Zig when running build-on-save.
+///
+/// If the `build.zig` has declared a 'check' step, it will be preferred over the default 'install' step.
+build_on_save_args: []const []const u8 = &.{},
 
 /// Whether to automatically fix errors on save. Currently supports adding and removing discards.
 enable_autofix: bool = false,

src/Server.zig

@@ -891,36 +891,39 @@ pub fn updateConfiguration(
         (server.config.build_runner_path == null or !std.mem.eql(u8, server.config.build_runner_path.?, new_config.build_runner_path.?));
 
     inline for (std.meta.fields(Config)) |field| {
-        if (@field(new_cfg, field.name)) |new_config_value| {
-            const old_config_value = @field(server.config, field.name);
-            switch (@TypeOf(old_config_value)) {
-                ?[]const u8 => {
-                    const override_old_value =
-                        if (old_config_value) |old_value| !std.mem.eql(u8, old_value, new_config_value) else true;
-                    if (override_old_value) {
-                        log.info("Set config option '{s}' to '{s}'", .{ field.name, new_config_value });
-                        @field(server.config, field.name) = try config_arena.dupe(u8, new_config_value);
-                    }
-                },
-                []const u8 => {
-                    if (!std.mem.eql(u8, old_config_value, new_config_value)) {
-                        log.info("Set config option '{s}' to '{s}'", .{ field.name, new_config_value });
-                        @field(server.config, field.name) = try config_arena.dupe(u8, new_config_value);
-                    }
-                },
-                else => {
-                    if (old_config_value != new_config_value) {
-                        switch (@typeInfo(@TypeOf(new_config_value))) {
-                            .bool,
-                            .int,
-                            .float,
-                            => log.info("Set config option '{s}' to '{}'", .{ field.name, new_config_value }),
-                            .@"enum" => log.info("Set config option '{s}' to '{s}'", .{ field.name, @tagName(new_config_value) }),
-                            else => @compileError("unexpected config type ++ (" ++ @typeName(@TypeOf(new_config_value)) ++ ")"),
+        if (@field(new_cfg, field.name)) |new_value| {
+            const old_value_maybe_optional = @field(server.config, field.name);
+
+            const override_value = blk: {
+                const old_value = if (@typeInfo(@TypeOf(old_value_maybe_optional)) == .optional)
+                    if (old_value_maybe_optional) |old_value| old_value else break :blk true
+                else
+                    old_value_maybe_optional;
+
+                break :blk switch (@TypeOf(old_value)) {
+                    []const []const u8 => {
+                        if (old_value.len != new_value.len) break :blk true;
+                        for (old_value, new_value) |old, new| {
+                            if (!std.mem.eql(u8, old, new)) break :blk true;
                         }
-                        @field(server.config, field.name) = new_config_value;
-                    }
-                },
+                        break :blk false;
+                    },
+                    []const u8 => !std.mem.eql(u8, old_value, new_value),
+                    else => old_value != new_value,
+                };
+            };
+
+            if (override_value) {
+                log.info("Set config option '{s}' to {}", .{ field.name, std.json.fmt(new_value, .{}) });
+                @field(server.config, field.name) = switch (@TypeOf(new_value)) {
+                    []const []const u8 => blk: {
+                        const copy = try config_arena.alloc([]const u8, new_value.len);
+                        for (copy, new_value) |*duped, original| duped.* = try config_arena.dupe(u8, original);
+                        break :blk copy;
+                    },
+                    []const u8 => try config_arena.dupe(u8, new_value),
+                    else => new_value,
+                };
             }
         }
     }
@@ -942,7 +945,7 @@ pub fn updateConfiguration(
         server.document_store.cimports.clearAndFree(server.document_store.allocator);
 
         if (std.process.can_spawn and
-            server.config.enable_build_on_save and
+            server.config.enable_build_on_save != false and
             server.client_capabilities.supports_publish_diagnostics)
         {
             try server.pushJob(.run_build_on_save);
@@ -1015,7 +1018,7 @@ pub fn updateConfiguration(
         }
     }
 
-    if (server.config.enable_build_on_save) {
+    if (server.config.enable_build_on_save orelse false) {
         if (!std.process.can_spawn) {
             log.info("'enable_build_on_save' is ignored because your OS can't spawn a child process", .{});
         } else if (server.status == .initialized and server.config.zig_exe_path == null) {
@@ -1351,7 +1354,7 @@ fn saveDocumentHandler(server: *Server, arena: std.mem.Allocator, notification:
     }
 
     if (std.process.can_spawn and
-        server.config.enable_build_on_save and
+        server.config.enable_build_on_save != false and
         server.client_capabilities.supports_publish_diagnostics)
     {
         try server.pushJob(.run_build_on_save);

src/features/diagnostics.zig

@@ -215,36 +215,72 @@ pub fn generateBuildOnSaveDiagnostics(
         },
     };
 
-    log.info("Running build-on-save: {s} ({s})", .{ build_zig_path, server.config.build_on_save_step });
+    const build_zig_uri = try URI.fromPath(server.allocator, build_zig_path);
+    defer server.allocator.free(build_zig_uri);
 
     const base_args = &[_][]const u8{
         zig_exe_path,
         "build",
-        server.config.build_on_save_step,
         "--zig-lib-dir",
         zig_lib_path,
         "-fno-reference-trace",
         "--summary",
         "none",
     };
 
-    var argv = try std.ArrayListUnmanaged([]const u8).initCapacity(arena, base_args.len);
+    var argv = try std.ArrayListUnmanaged([]const u8).initCapacity(arena, base_args.len + server.config.build_on_save_args.len);
     defer argv.deinit(arena);
     argv.appendSliceAssumeCapacity(base_args);
+    argv.appendSliceAssumeCapacity(server.config.build_on_save_args);
+
+    const has_explicit_steps = for (server.config.build_on_save_args) |extra_arg| {
+        if (!std.mem.startsWith(u8, extra_arg, "-")) break true;
+    } else false;
+
+    var has_check_step: bool = false;
 
     blk: {
         server.document_store.lock.lockShared();
         defer server.document_store.lock.unlockShared();
-        const build_file = server.document_store.build_files.get(build_zig_path) orelse break :blk;
-        const build_associated_config = build_file.build_associated_config orelse break :blk;
-        const build_options = build_associated_config.value.build_options orelse break :blk;
+        const build_file = server.document_store.build_files.get(build_zig_uri) orelse break :blk;
+
+        no_build_config: {
+            const build_associated_config = build_file.build_associated_config orelse break :no_build_config;
+            const build_options = build_associated_config.value.build_options orelse break :no_build_config;
+
+            try argv.ensureUnusedCapacity(arena, build_options.len);
+            for (build_options) |build_option| {
+                argv.appendAssumeCapacity(try build_option.formatParam(arena));
+            }
+        }
 
-        try argv.ensureUnusedCapacity(arena, build_options.len);
-        for (build_options) |build_option| {
-            argv.appendAssumeCapacity(try build_option.formatParam(arena));
+        no_check: {
+            if (has_explicit_steps) break :no_check;
+            const config = build_file.tryLockConfig() orelse break :no_check;
+            defer build_file.unlockConfig();
+            for (config.top_level_steps) |tls| {
+                if (std.mem.eql(u8, tls, "check")) {
+                    has_check_step = true;
+                    break;
+                }
+            }
         }
     }
 
+    if (!(server.config.enable_build_on_save orelse has_check_step)) {
+        return;
+    }
+
+    if (has_check_step) {
+        std.debug.assert(!has_explicit_steps);
+        try argv.append(arena, "check");
+    }
+
+    const extra_args_joined = try std.mem.join(server.allocator, " ", argv.items[base_args.len..]);
+    defer server.allocator.free(extra_args_joined);
+
+    log.info("Running build-on-save: {s} ({s})", .{ build_zig_uri, extra_args_joined });
+
     const result = std.process.Child.run(.{
         .allocator = server.allocator,
         .argv = argv.items,

src/tools/config.json

@@ -14,15 +14,15 @@
         },
         {
             "name": "enable_build_on_save",
-            "description": "Whether to enable build-on-save diagnostics",
-            "type": "bool",
-            "default": false
+            "description": "Whether to enable build-on-save diagnostics. Will be automatically enabled if the `build.zig` has declared a 'check' step.",
+            "type": "?bool",
+            "default": null
         },
         {
-            "name": "build_on_save_step",
-            "description": "Select which step should be executed on build-on-save",
-            "type": "[]const u8",
-            "default": "install"
+            "name": "build_on_save_args",
+            "description": "Specify which arguments should be passed to Zig when running build-on-save.\n\nIf the `build.zig` has declared a 'check' step, it will be preferred over the default 'install' step.",
+            "type": "[]const []const u8",
+            "default": []
         },
         {
             "name": "enable_autofix",

src/tools/config_gen.zig

@@ -16,15 +16,17 @@ const ConfigOption = struct {
     default: std.json.Value,
 
     fn getTypescriptType(self: ConfigOption) error{UnsupportedType}![]const u8 {
-        return if (std.mem.eql(u8, self.type, "[]const u8"))
+        std.debug.assert(self.type.len != 0);
+        const ty = self.type[@intFromBool(self.type[0] == '?')..];
+        return if (std.mem.eql(u8, ty, "[]const []const u8"))
+            "array"
+        else if (std.mem.eql(u8, ty, "[]const u8"))
             "string"
-        else if (std.mem.eql(u8, self.type, "?[]const u8"))
-            "string"
-        else if (std.mem.eql(u8, self.type, "bool"))
+        else if (std.mem.eql(u8, ty, "bool"))
             "boolean"
-        else if (std.mem.eql(u8, self.type, "usize"))
+        else if (std.mem.eql(u8, ty, "usize"))
             "integer"
-        else if (std.mem.eql(u8, self.type, "enum"))
+        else if (std.mem.eql(u8, ty, "enum"))
             "string"
         else
             error.UnsupportedType;
@@ -62,6 +64,15 @@ const ConfigOption = struct {
     ) !void {
         _ = options;
         if (fmt.len != 0) return std.fmt.invalidFmtError(fmt, ConfigOption);
+        if (config.default == .array) {
+            try writer.writeAll("&.{");
+            for (config.default.array.items, 0..) |item, i| {
+                if (i != 0) try writer.writeByte(',');
+                try std.json.stringify(item, .{}, writer);
+            }
+            try writer.writeByte('}');
+            return;
+        }
         if (config.@"enum" != null) {
             try writer.print(".{s}", .{config.default.string});
             return;
@@ -89,19 +100,47 @@ const Schema = struct {
 const SchemaEntry = struct {
     description: []const u8,
     type: []const u8,
+    items: ?struct { type: []const u8 } = null,
     @"enum": ?[]const []const u8 = null,
     default: std.json.Value,
 };
 
-fn generateConfigFile(allocator: std.mem.Allocator, config: Config, path: []const u8) !void {
-    _ = allocator;
+fn formatDocs(
+    text: []const u8,
+    comptime fmt: []const u8,
+    options: std.fmt.FormatOptions,
+    writer: anytype,
+) @TypeOf(writer).Error!void {
+    _ = options;
+    if (fmt.len != 1) std.fmt.invalidFmtError(fmt, text);
+    const prefix = switch (fmt[0]) {
+        'n' => "// ",
+        'd' => "/// ",
+        '!' => "//! ",
+        else => std.fmt.invalidFmtError(fmt, text),
+    };
+    var i: usize = 0;
+    var iterator = std.mem.splitScalar(u8, text, '\n');
+    while (iterator.next()) |line| : (i += 1) {
+        if (i != 0) try writer.writeByte('\n');
+        try writer.print("{s}{s}", .{ prefix, line });
+    }
+}
 
-    const config_file = try std.fs.cwd().createFile(path, .{});
-    defer config_file.close();
+/// The format specifier must be one of:
+///  * `{n}` writes normal (`//`) comments.
+///  * `{d}` writes doc-comments (`///`) comments.
+///  * `{!}` writes top-level-doc-comments (`//!`) comments.
+fn fmtDocs(text: []const u8) std.fmt.Formatter(formatDocs) {
+    return .{ .data = text };
+}
 
-    var buff_out = std.io.bufferedWriter(config_file.writer());
+fn generateConfigFile(allocator: std.mem.Allocator, config: Config, path: []const u8) (std.fs.Dir.WriteFileError || std.mem.Allocator.Error)!void {
+    var buffer = std.ArrayList(u8).init(allocator);
+    defer buffer.deinit();
+    const writer = buffer.writer();
 
-    _ = try buff_out.write(
+    try writer.writeAll(
         \\//! DO NOT EDIT
         \\//! Configuration options for ZLS.
         \\//! If you want to add a config option edit
@@ -111,26 +150,39 @@ fn generateConfigFile(allocator: std.mem.Allocator, config: Config, path: []cons
     );
 
     for (config.options) |option| {
-        try buff_out.writer().print(
+        try writer.print(
             \\
-            \\/// {s}
+            \\{d}
             \\{}: {} = {},
             \\
         , .{
-            std.mem.trim(u8, option.description, &std.ascii.whitespace),
+            fmtDocs(std.mem.trim(u8, option.description, &std.ascii.whitespace)),
             std.zig.fmtId(std.mem.trim(u8, option.name, &std.ascii.whitespace)),
             option.fmtZigType(),
             option.fmtDefaultValue(),
         });
     }
 
-    _ = try buff_out.write(
+    _ = try writer.writeAll(
         \\
         \\// DO NOT EDIT
         \\
     );
 
-    try buff_out.flush();
+    const source_unformatted = try buffer.toOwnedSliceSentinel(0);
+    defer allocator.free(source_unformatted);
+
+    var tree = try std.zig.Ast.parse(allocator, source_unformatted, .zig);
+    defer tree.deinit(allocator);
+    std.debug.assert(tree.errors.len == 0);
+
+    buffer.clearRetainingCapacity();
+    try tree.renderToArrayList(&buffer, .{});
+
+    try std.fs.cwd().writeFile(.{
+        .sub_path = path,
+        .data = buffer.items,
+    });
 }
 
 fn generateSchemaFile(allocator: std.mem.Allocator, config: Config, path: []const u8) !void {
@@ -148,6 +200,7 @@ fn generateSchemaFile(allocator: std.mem.Allocator, config: Config, path: []cons
         schema.properties.map.putAssumeCapacityNoClobber(option.name, .{
             .description = option.description,
             .type = try option.getTypescriptType(),
+            .items = if (std.mem.eql(u8, option.type, "[]const []const u8")) .{ .type = "string" } else null,
             .@"enum" = option.@"enum",
             .default = option.default,
         });
@@ -223,7 +276,8 @@ fn generateVSCodeConfigFile(allocator: std.mem.Allocator, config: Config, path:
             .description = option.description,
             .@"enum" = option.@"enum",
             .format = if (std.mem.indexOf(u8, option.name, "path") != null) "path" else null,
-            .default = default,
+            // "enable_build_on_save" need to be explicitly set to 'null' so that it doesn't default to 'false'
+            .default = default orelse .null,
         });
     }