ryanluker / vscode-coverage-gutters

Display test coverage generated by lcov and xml - works with many languages
https://marketplace.visualstudio.com/items?itemName=ryanluker.vscode-coverage-gutters
MIT License
454 stars 88 forks source link

`${workspaceFolder}` not supported in manualCoverageFilePaths #396

Closed garymm closed 1 year ago

garymm commented 1 year ago

Describe the bug I have a coverage file in some subdirectory of my workspace.

When I specify the path as relative to my workspace, or using ${workspaceFolder}, the extension is not able to load the file. It fails with errors like ENOENT: no such file or directory, open ...

If I replace ${workspaceFolder} with the absolute path to my workspace, the extension is able to load the file.

E.g.:

This works:

"coverage-gutters.manualCoverageFilePaths": ["/foo/bar/workspace/coverage.dat"]

This doesn't:

"coverage-gutters.manualCoverageFilePaths": ["coverage.dat"]

This doesn't:

"coverage-gutters.manualCoverageFilePaths": ["${workspaceFolder}/coverage.dat"]

Desktop (please complete the following information):

Additional context Thanks for the extension!

ryanluker commented 1 year ago

@garymm Thanks for the ticket, I am not familiar with ${workspaceFolder} is this something that the newer vscode api's provide to the extension settings? (@mattseddon might have insights here as well if the extension needs to consider some template like variables?)

mattseddon commented 1 year ago

Visual Studio Code supports variable substitution in Debugging and Task configuration files as well as some select settings. Variable substitution is supported inside some key and value strings in launch.json and tasks.json files using ${variableName} syntax.

source https://code.visualstudio.com/docs/editor/variables-reference

You could make it work but it would definitely not be free.

The best (quickest) option for low maintenance is to specify in the docs/description of the config option (coverage-gutters.manualCoverageFilePaths) that the path provided must be a full path. I don't think this would lead to terrible UX because the option only has to be set once.

ryanluker commented 1 year ago

Visual Studio Code supports variable substitution in Debugging and Task configuration files as well as some select settings. Variable substitution is supported inside some key and value strings in launch.json and tasks.json files using ${variableName} syntax.

source https://code.visualstudio.com/docs/editor/variables-reference

You could make it work but it would definitely not be free.

The best (quickest) option for low maintenance is to specify in the docs/description of the config option (coverage-gutters.manualCoverageFilePaths) that the path provided must be a full path. I don't think this would lead to terrible UX because the option only has to be set once.

Thanks @mattseddon sounds like a good option on the documentation front.

josephzidell commented 1 year ago

This unfortunately precludes sharing these settings with a team. We're actually running into this issue right now, where we want to put these setting into our versioned ${repo}/.vscode/settings.json like we do with certain other settings ("eslint.workingDirectories", "java.checkstyle.version", etc.).

Are we open to accepting a PR to support either ./... or ${workspaceFolder}/...?

mattseddon commented 1 year ago

I can't speak for @ryanluker but I would say that if you are sharing settings/extensions with a team then a good option is to create a shared dev container.

https://code.visualstudio.com/docs/devcontainers/create-dev-container

ryanluker commented 1 year ago

This unfortunately precludes sharing these settings with a team. We're actually running into this issue right now, where we want to put these setting into our versioned ${repo}/.vscode/settings.json like we do with certain other settings ("eslint.workingDirectories", "java.checkstyle.version", etc.).

Are we open to accepting a PR to support either ./... or ${workspaceFolder}/...?

@josephzidell Thanks for the input! The repo is very open for PRs to add extra support as long as the tests pass and the performance isn't worsened for the other path use cases.

Take a look at @mattseddon 's suggestion first though and see if that fills your needs 🤔.