Skip to content

feat: port readDirectoryRecursively from VSC #923

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

Merged
merged 9 commits into from
Apr 9, 2025

Conversation

Hweinstock
Copy link
Contributor

@Hweinstock Hweinstock commented Apr 8, 2025

Problem

This utility function is needed for the agentic tools and currently lives in the VSC repo.

Solution

  • Port it to the flare. (Source)
  • Add tests for it (currently missing in VSC).
  • refactor. Only major difference is that it no longer pushes stringified-errors to the results, and allows an option to fail on errors.

Notes

This PR contains some workarounds with TODO comments that can be properly addressed by aws/language-server-runtimes#415.

This same PR will also fix the warning on newer versions of node:

(node:40516) [DEP0178] DeprecationWarning: dirent.path is deprecated in favor of dirent.parentPath
(Use `node --trace-deprecation ...` to show where the warning was created

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@Hweinstock Hweinstock marked this pull request as ready for review April 8, 2025 15:18
@Hweinstock Hweinstock requested a review from a team as a code owner April 8, 2025 15:18
Comment on lines +22 to +27
testFeatures.workspace.fs.readdir = path => fs.readdir(path, { withFileTypes: true })
testFeatures.workspace.fs.exists = path =>
fs.access(path).then(
() => true,
() => false
)
Copy link
Contributor

Choose a reason for hiding this comment

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

TestFeatures already provides stubs for all namespaces, so this should technically not be necessary. Unless you want to 'undo' the stubbing and go back to the real filesystem.

See also: https://github.com/aws/language-server-runtimes/blob/674c02696c150838b4bc93543fb0009c5982e7ad/runtimes/testing/TestFeatures.ts#L53

Copy link
Contributor

@kmile kmile Apr 8, 2025

Choose a reason for hiding this comment

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

Looking at the rest of the test, it's indeed easier just to defer to the actual filesystem than to stub out a bunch of recursive / nested calls to TestFeatures. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I am intending to 'undo' the stubbing here, but most ways felt pretty awkward. This is the best solution I came up with, but lmk if you see a better option.

@Hweinstock Hweinstock merged commit af48204 into aws:main Apr 9, 2025
6 checks passed
@Hweinstock Hweinstock deleted the util/readDirectoryRecursively branch April 9, 2025 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants