Skip to content

Fix filepaths with space#540

Open
soda0289 wants to merge 6 commits into
bazel-contrib:masterfrom
soda0289:fix-filepaths-with-space
Open

Fix filepaths with space#540
soda0289 wants to merge 6 commits into
bazel-contrib:masterfrom
soda0289:fix-filepaths-with-space

Conversation

@soda0289

@soda0289 soda0289 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

I found an issue where if a file path has a space "foo/api tests/some_test.cs" then the compile would fail. This seems to only happen with csharp compiler. I think the fsharp compiler sends each cli argument as a new line which makes it support spaces in folder and file names.

I can add more tests as well to test each possible case. First time looking into this code base maybe there's a better way to handle it as well.

@soda0289 soda0289 requested a review from purkhusid as a code owner June 8, 2026 20:02
@purkhusid

Copy link
Copy Markdown
Collaborator

Thanks for the fix! I think the e2e tests are not necessary since they are more of a smoke test. If you could remove the e2e tests then I'll merge the fix!

@soda0289 soda0289 force-pushed the fix-filepaths-with-space branch from d8251f1 to 7a7d8f9 Compare June 9, 2026 19:46
@soda0289

soda0289 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Remove e2e tests and added a few more unit tests for resources and references with whitespace.

@purkhusid

Copy link
Copy Markdown
Collaborator

Don't worry about the failed Buildifier. I'll fix that after I merge.
One more thing. Does F# fail with the same path handling? If not then I would prefer that the path handling just also be applied there to keep the differences between C# and F# as small as possible.

@soda0289

soda0289 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

I applied the same whitespace handling to F#. There is no issue adding it in and it also allows us to move the format_arg functions into common.bzl in private and remove some conditions that were only for csharp. Let me know if I should change anything else.

@soda0289

Copy link
Copy Markdown
Contributor Author

After doing more testing I found that the fsharp compiler doesn't strip the double quotes it includes them in the output. Will revert change and add tests for fsharp to assert no quotes are added.

@soda0289 soda0289 force-pushed the fix-filepaths-with-space branch from 99a5960 to b22b8ba Compare June 10, 2026 15:57
@soda0289

Copy link
Copy Markdown
Contributor Author

Pushed the change to ensure F# compiler doesn't add quotes. Quotes do work for source file but not anything else.

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