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

Add test case and fix for annotation processor option with space #272

Closed
wants to merge 5 commits into from

Conversation

guw
Copy link
Contributor

@guw guw commented Aug 29, 2023

Add a test to reproduce bazelbuild/bazel#19360

@guw guw force-pushed the main branch 3 times, most recently from 86174eb to b983a53 Compare August 29, 2023 15:39
Copy link
Collaborator

@cushon cushon left a comment

Choose a reason for hiding this comment

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

I think the issue with the existing params file logic is the use of ARG_SPLITTER.split(Files.readString(paramsPath)). ARG_SPLITTER splits on 'breaking whitespace', which includes both spaces and newlines, and it seems like for params files we should only be splitting on newlines.

I wonder if we could replace the use of ARG_SPLITTER.split(Files.readString(paramsPath)) with Files.readAllLines(paramsPath) instead, do you see issues with that approach?

It would also avoid changing the escaping logic discussed in the other comment thread.

java/com/google/turbine/options/TurbineOptionsParser.java Outdated Show resolved Hide resolved
@guw guw requested a review from cushon September 11, 2023 11:28
@guw guw changed the title Add test case for annotation processor option with space Add test case and fix for annotation processor option with space Sep 11, 2023
copybara-service bot pushed a commit that referenced this pull request Sep 11, 2023
Add a test to reproduce bazelbuild/bazel#19360

Fixes #272

FUTURE_COPYBARA_INTEGRATE_REVIEW=#272 from guw:main b2b7c2b
PiperOrigin-RevId: 564393335
treblereel pushed a commit to treblereel/turbine that referenced this pull request Sep 28, 2023
Add a test to reproduce bazelbuild/bazel#19360

Fixes google#272

COPYBARA_INTEGRATE_REVIEW=google#272 from guw:main b2b7c2b
PiperOrigin-RevId: 564447484
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