[BUG] Dialog: "" not found. Configure verilog.ctags.path, add to PATH, or disable in config.

the-moog commented 3 months ago

Describe the bug - A clear and concise description of what the bug is.

There appears to be a problem with the way this extension works with the vscode config parser. In that, unlike other extensions, it is storing the settings as quoted text representing YAML, rather than YAML itself.

Settings for paths that are altered from default look like this on disk:

"verilog.ctags.path": "{\"windows\":\"ctags.exe\",\"linux\":\"~/.local/bin/ctags\",\"mac\":\"ctags\"}",

Whereas they should look like this:

"verilog.ctags.path": {"windows":"ctags.exe","linux":"~/.local/bin/ctags","mac":"ctags"},

In the above, note the escaped quotes \"....\" and quoted value string "{....}" in the first example. If changed to the canonical form in the 2nd example the settings editor throws an error. Note: The rest of the relevant extenion settings are attached below.

Perhaps the schema for the config entries is incorrect? It wants string and it should be a yaml structure. As it is a string (containing a line of yaml text) could it be being misinterpreted as a path, causing the error seen in this issue?

I also think this is true for all the extension settings that require a path and not just ctags I am reporting here. Though I don't have most of them so can't be sure

Other related issues (I can split these out as separate related issues as required):

I am guessing here, likely because of this incorrect parsing it is not seeing the POSIX shell tilde ~, usually meaning ${HOME} or similar. If that is not the case then this should be reviewed as this works with other paths in vscode. This is essential as remote systems don't always have absolute global paths or guarantee passing environment variables when projects are shared between multiple users.

Root cause - vague setting names?

Perhaps the root cause is that the settings dialog is inconsistent with the use of path as this is unclear if this means executable path, tool location or executable filename path. I would suggest making this clearer by renaming the dialog prompt and setting keys consistently, such that 'path' accepts either a prefix or a full filename path. And where it must and can only be a path, use something like tool_path_prefix, as that settings only accepts a path as it points to more than one file. Pointing it to a file should be an error, though I don't know if the schema checker can express that? Having one setting that means both prefix and name could be a problem or cause odd things to happen. Perhaps just separate it as shown below.

Specifically: Conforming to the way similar standard vscode settings work, when providing a path it should resolve to YAML the value of which accepts an optional platform qualifier, e.g. string "ctags_executable": "ctags"

or - per platform using a key-value pair "ctags_executable": {"linux": "ctags"}

And (again following conventions) similar settings where appropriate, this should accept empty "", meaning a built-in implicit 'standard' filename (in this case "ctags") with an optional user overriding file name, a user prefix path to the standard name or an explicit full path to an executable, e.g. Alternate executable "ctags_executable": "myctags"

or - implied "ctags" executable "ctags_executable": "~/path/to/localbuilds"

or - explicit "myctags" executable "ctags_executable": "~/local/bin/myctags"

Extension settings (and logging/extension host) prefix.

The YAML key verilog.ctags.path is being set globally, through the name 'verilog', therefore it conflicts with other similar extensions if they also start with "verilog......". This is true of the extension host too. Perhaps change 'verilog' to the name of this repo, andrewnolte.vscode-system-verilog or something similar, so that settings are within a container specific to this extension. Now the above examples would look something like this:

"andrewnolte.vscode-system-verilog": {
  "ctags.path": "ctags",
  "ctags.executable: "",
  ....
}

Environment:

Linux Ubuntu 22.04 (remote)
Win11 Host, latest VSCode
config: see code block below

{
    "verilog.ctags.indexAllIncludes": true,
    "verilog.lint.slang.enabled": true,
    "verilog.ctags.path": "{\"windows\":\"ctags.exe\",\"linux\":\"~/.local/bin/ctags\",\"mac\":\"ctags\"}",
    "verible.path": "~/.local/bin",
    "verilog.languageServer.veribleVerilogLs.path": "~/.local/bin/verible-verilog-ls",
    "verilog.languageServer.veribleVerilogLs.enabled": true,
    "verilog.verilogStandard": "Verilog-2005",
    "verilog.lint.slang.path": "{\"windows\":\"slang.exe\",\"linux\":\"~/.local/bin/slang\",\"mac\":\"slang\"}",
    "verilog.lint.verilator.enabled": true,
    "verilog.svFormat.verible.path": "{\"windows\":\"verible-verilog-format.exe\",\"linux\":\"~/.local/bin/verible-verilog-format\",\"mac\":\"verible-verilog-format\"}",
    "verilog.svStandard": "SystemVerilog-2017"
}

Steps to reproduce:

change the setting for ctags on remote workspace I have no idea if this issue is only on a remote or if the same is true for local and/or workspace settings.

Logs: (Get logs from Output > verilog)

There is little or nothing to add from the system log or the extension log. I can send this if required but it has to be sanitised due to corporate rules about sharing internal information.

I get these popup dialogs, repeated every time a HDL file goes into scope.

X
"" not found. Configure verilog.ctags.path, add to PATH, or disable in config.

Source: Verilog/SystemVerilog Tools

the-moog commented 3 months ago

Related issue. The config file schema checker does not like "verilog.svStandard": "SystemVerilog-2017" it says the value field should be an enumeration, but only lets you enter a string.

AndrewNolte commented 3 months ago

Thanks for the feedback!

The first one is a bit confusing. It's intended to be a path for the platform you're on (no json), but I specify the default as that to show that it'll choose the correct one for your platform. To fix I'll just have it show no default avoid this confusion, and mention the defaults in CONFIG.md

There doesn't seem to be a way to specify that this is the case right now in package.json https://code.visualstudio.com/api/references/contribution-points#contributes.configuration. The vscode extension API is quite a mess. The defaults specified in package.json aren't actually used by the config api, they get respecified in code, which is why I made a library to generate the package.json from the code.

The reason the tilde doesn't work is because I changed it to no longer execute the commands in a shell. Is this really necessary for your setup?

With the followup comment, this is just a issue with the json config; it will parse it correctly. I'll fix it so it doesn't show an error in the json.

AndrewNolte commented 3 months ago

I just published a fix for the confusing default and the enum config schema. Can you reopen individual issues if there's still any, and make them a bit more concise? As I was rereading it seems like you assumed a lot of things that aren't true. For example, it's json, there's no yaml.

As I was rereading I saw your comment about the overlapping configs. It does overlap with https://github.com/mshr-h/vscode-verilog-hdl-support on the path configs, but the paths shouldn't need to be different. If the absolute path can't be specified, you can put it on your PATH and it'll find the abs path when it activates, and the old extension should work with that as well. You should only be using one or the other though, and I'd encourage having everyone on your project decide on one.

AndrewNolte / vscode-system-verilog