Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Files in imports / current workspace aren't watched #105

Open
kubukoz opened this issue Oct 7, 2022 · 1 comment
Open

Files in imports / current workspace aren't watched #105

kubukoz opened this issue Oct 7, 2022 · 1 comment
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@kubukoz
Copy link
Owner

kubukoz commented Oct 7, 2022

We should reload the workspace when something in these files changes. Currently a reload has to be triggered by changing the build itself, e.g. reordering the keys in a list or removing one of the keys in the main object.

OUTDATED: Additionally, if imports is not set (as opposed to empty), we won't see any specs - what should happen instead is loading the entire workspace directory.

Update on the above: I don't think it makes any sense - we'd have to basically sift through all .smithy files in the workspace, then also add all .json files that happen to have a smithy member containing the language version (requires actually reading and parsing the files). I think a simpler solution for the time being will be to require an explicit imports list.

@kubukoz kubukoz added the bug Something isn't working label Oct 16, 2022
@kubukoz
Copy link
Owner Author

kubukoz commented Oct 16, 2022

The fix will have two parts:

Part 1:

The pattern for file watching is defined here:

fileEvents: workspace.createFileSystemWatcher(
"**/{build/smithy-dependencies.json,.smithy.json,smithy-build.json}"
),

so it doesn't take into account the files imported by the build definition. I don't think we should be watching arbitrary paths on disk though, so I'd say the fix for this would involve adding a file watcher for .smithy and .json files inside the workspace. (worth checking if Smithy tooling has support for any other alternate syntaxes but I think that's it).

This can be a separate PR as it doesn't break existing functionality - as long as it goes first.

Part 2:

Once the files are being watched, the handler of didChangeWatchedFiles needs to be made aware of these files. Currently isChanged is a property defined here:

BuildLoader[F].load.map { case params =>
PrepareResult(params, !lastUsedConfig.contains(params.config))
}

BuildLoader[F].load only knows about the parsed contents of the build file, and it should stay that way. I think the right place to read & compare the files would be in ServerLoader still, and it should be based on a hash of the (recursive) contents of all files in params.config.imports.

@kubukoz kubukoz added the good first issue Good for newcomers label Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant