Do not allow *.path options to be null

[Vexu]

Allowing the values to be null disables setting the values in the settings UI.

[Techatrix]

Allowing the values to be null disables setting the values in the settings UI.

I really to tried to test my changes as much as possible but using the settings UI never crossed my mind...

---

There are still many locations where get<string | null>("path", null) happens. Those need to be adjusted for this change.

Last time I checked, the default value for a string option is not undefined but "" which means that the following change should be applied in most cases.

- configuration.get<string | null>("path", null);
+ configuration.get<string>("path") ?? null;
+ configuration.get<string>("path") ?? undefined; // when calling getExePath 

Most importantly, this code needs to be adjusted or removed entirely.

I'd be happy to work on these changes myself if you'd like.

[Vexu]

There are still many locations where get<string | null>("path", null) happens. Those need to be adjusted for this change.

get<string>(...) returns string | undefined using get<string | null>(..., null) just changes the undefined to null.

Last time I checked, the default value for a string option is not undefined but "" which means that the following change should be applied in most cases.

I'd expect the opposite to be true but I'll have to check.

[Vexu]

Setting the default value to null on a string option seems to work:

diff --git a/package.json b/package.json
index 5b9164c..5e80a93 100644
--- a/package.json
+++ b/package.json
@@ -68,6 +68,11 @@
       "type": "object",
       "title": "Zig",
       "properties": {
+        "zig.testOption": {
+          "type": "string",
+          "default": null,
+          "description": "just testing"
+        },
         "zig.initialSetupDone": {
           "type": "boolean",
           "default": false,
diff --git a/src/zigSetup.ts b/src/zigSetup.ts
index 8ab9314..9fd3908 100644
--- a/src/zigSetup.ts
+++ b/src/zigSetup.ts
@@ -264,6 +264,9 @@ export async function setupZig(context: vscode.ExtensionContext) {
     );
 
     const configuration = vscode.workspace.getConfiguration("zig");
+    void vscode.window.showInformationMessage(`testOption=${(configuration.get<string | null>("testOption", null) === null).toString()}`, {
+        modal: true,
+    });
     if (!configuration.get<boolean>("initialSetupDone")) {
         await configuration.update("initialSetupDone", await initialSetup(context), true);
     }

testOption=true

[Techatrix]

Setting the default value to null on a string option seems to work:

Doesn't this bring us back to #166 again?

[Vexu]

I don't think so? getExePath should work with both null and zig, and the empty string to exe name conversion should take care of the empty string case.

[Techatrix]

I mean that having null as the default value will make VS Code show a warning because null is not a valid string which was one of the issues that the PR I mentioned wanted to fix.

"zig.testOption": {
  "type": "string",
  "default": null,
  "description": "just testing"
},

But perhaps there is a way to work around this

[Vexu]

I wouldn't expect anyone to actually set the option with null but I guess I'll have to do it the javascript way and treat empty string as null.