[lldb-dap] show dialog when executable is not found

[da-viper]

Fixes #103043

[llvmbot]

@llvm/pr-subscribers-lldb

Author: None (Da-Viper)

Changes

Fixes #103043

---

Full diff: https://github.com/llvm/llvm-project/pull/104711.diff

1 Files Affected:

• (modified) lldb/tools/lldb-dap/src-ts/extension.ts (+26-3)

diff --git a/lldb/tools/lldb-dap/src-ts/extension.ts b/lldb/tools/lldb-dap/src-ts/extension.ts
index 791175f7b46224..c7802eed2a513b 100644
--- a/lldb/tools/lldb-dap/src-ts/extension.ts
+++ b/lldb/tools/lldb-dap/src-ts/extension.ts
@@ -1,4 +1,5 @@
 import * as vscode from "vscode";
+import * as fs from "node:fs/promises";
 import { LLDBDapOptions } from "./types";
 import { DisposableContext } from "./disposable-context";
 import { LLDBDapDescriptorFactory } from "./debug-adapter-factory";
@@ -17,10 +18,32 @@ function createDefaultLLDBDapOptions(): LLDBDapOptions {
       const path = vscode.workspace
         .getConfiguration("lldb-dap", session.workspaceFolder)
         .get<string>("executable-path");
-      if (path) {
-        return new vscode.DebugAdapterExecutable(path, []);
+
+      if (!path) {
+        return packageJSONExecutable;
+      }
+
+      try {
+        const fileStats = await fs.stat(path);
+        if (!fileStats.isFile()) {
+          throw new Error(`Error: ${path} is not a file`);
+        }
+      } catch (err) {
+        const error: Error = err as Error;
+        const openSettingsAction = "Open Settings";
+        const callBackValue = await vscode.window.showErrorMessage(
+          error.message,
+          { modal: true },
+          openSettingsAction,
+        );
+        if (openSettingsAction === callBackValue) {
+          vscode.commands.executeCommand(
+            "workbench.action.openSettings",
+            "lldb-dap.executable-path",
+          );
+        }
       }
-      return packageJSONExecutable;
+      return new vscode.DebugAdapterExecutable(path, []);
     },
   };
 }

[JDevlieghere]

I'm not a TS expert but functionally this is a great improvement. LGTM.

[vogelsgesang]

Thanks for looking into this! The patch already looks very good!

[walter-erquinigo]

@da-viper , a better way to do this is that, instead of checking for the path upon initialization, you add this checking logic in the createDebugAdapterDescriptor function, which only gets triggered when you launch the debugger. This will be less annoying to users who have the extension but rarely use it.

[walter-erquinigo]

cool

Don't forget to run npm run format before landing this

[da-viper]

cool

Don't forget to run npm run format before landing this

Done

[vogelsgesang]

Looks good to me!
Thanks for this amazing usability improvement!
If this would have existed the first time I tried this extension, it would have saved me a lot of time

[walter-erquinigo]

Suggested change

	const path: string = customPath ? customPath : executable!!.command;
	const path: string = customPath || executable!!.command;

[walter-erquinigo]

What if validateDebugAdapterPath returns a boolean, and then, in the case of success, you return createDapExecutableCommand, and otherwise you throw an exception?
I have the impression that that would be managed better by the IDE, otherwise you could be launching a session with a missing binary

[walter-erquinigo]

I like this!

[vogelsgesang]

Looks good to me! Thanks again for fixing this!

[da-viper]

This is what is shown when I return undefined from the createDebugAdapterDescriptor function

If I also throw vscode.FileSystemError.FileExists(`Debug path: ${path} is not vaild`)
It shows the same as above but with the file path as a description

[walter-erquinigo]

Thanks for the screenshot, man!

[JDevlieghere]

Looks like this is ready to go. @da-viper do you need someone to merge this for you?

[tedwoodward]

Does the version number in package.json need to be updated, to automatically publish the change to the marketplace?

[da-viper]

Looks like this is ready to go. @da-viper do you need someone to merge this for you?

Yes, As I do not have write permission to the repository