Skip to content

Conversation

@davidsneighbour
Copy link

see #66

this allows use of relative path to the config file and variable use and makes the following valid configuration options:

{
  vale.valeCLI.config: /home/patrick/github.com/davidsneighbour/kollitsch.dev/src/config/vale/vale.ini,
  vale.valeCLI.config: /src/config/vale/vale.ini,
  vale.valeCLI.config: ./src/config/vale/vale.ini,
}

see ChrisChinchilla#66

this allows use of relative path to the config file and  variable use and makes the following valid configuration options:

{
  vale.valeCLI.config: /home/patrick/github.com/davidsneighbour/kollitsch.dev/src/config/vale/vale.ini,
  vale.valeCLI.config: /src/config/vale/vale.ini,
  vale.valeCLI.config: ./src/config/vale/vale.ini,
}

Signed-off-by: Patrick Kollitsch <[email protected]>
@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Jun 29, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes the resolution of the workspace folder in the config path, ensuring better compatibility when using relative paths or variable substitution. The changes introduce logic to resolve "${workspaceFolder}" strings and join relative paths with the workspace root.

  • Add logic to retrieve and resolve the config path.
  • Replace the direct configuration value with a resolved config object.
  • Support both absolute and relative path formats.
Comments suppressed due to low confidence (1)

src/lsp.ts:170

  • Ensure that unit tests are updated to cover the new relative path resolution logic for the config path.
    configPath: { value: resolvedConfigPath } as valeArgs,

@ChrisChinchilla
Copy link
Owner

Thanks @davidsneighbour I need to test this a bit as I don't really use workspaces much (also helps me understand the code for the future) but will do it asap. I am planning a new release soon anyway.

@ChrisChinchilla ChrisChinchilla self-assigned this Jul 2, 2025
Copy link
Owner

@ChrisChinchilla ChrisChinchilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added details to readme and settings UI, but looks good! I don't think I ever saw this as I have always had complex series of symlinks to ini files.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jul 7, 2025
@ChrisChinchilla ChrisChinchilla merged commit e1bb080 into ChrisChinchilla:main Jul 7, 2025
4 checks passed
@dosubot dosubot bot mentioned this pull request Jul 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants