Skip to content

Conversation

@ahoppen
Copy link
Member

@ahoppen ahoppen commented Jan 17, 2024

Previously, we were splitting command line arguments on Windows using the same rules as on Unix, which was incorrect, most importantly because backslashes in the first component of a Windows command line invocation are not escaping anything but interpreted verbatim.

The unix command line splitting remains completely unchanged, I just moved it to a different files. Similarly, the assertions for Unix command line splitting remain the same, I just moved the test cases and restructured them slightly.

Fixes #1020
rdar://120809063

@ahoppen ahoppen requested a review from benlangmuir as a code owner January 17, 2024 23:01
@ahoppen ahoppen requested a review from bnbarham January 17, 2024 23:01
@ahoppen
Copy link
Member Author

ahoppen commented Jan 17, 2024

@swift-ci Please test

Previously, we were splitting command line arguments on Windows using the same rules as on Unix, which was incorrect, most importantly because backslashes in the first component of a Windows command line invocation are not escaping anything but interpreted verbatim.

Fixes swiftlang#1020
rdar://120809063
@ahoppen ahoppen force-pushed the ahoppen/split-windows-command-line branch from b4573e9 to cfea672 Compare January 18, 2024 06:14
@ahoppen
Copy link
Member Author

ahoppen commented Jan 18, 2024

@swift-ci Please test

@MaxDesiatov
Copy link
Contributor

@swift-ci test windows

@ahoppen
Copy link
Member Author

ahoppen commented Jan 18, 2024

@swift-ci Please test Windows

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

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

Going to be honest, I mostly glazed over the actually implementation (especially because most of it is red due to the syntax highlighting currently). The tests looked mostly reasonable from the comments though.

Comment on lines +116 to +117
assertEscapedCommand(#"" test with "" quote""#, [" test with quote"], windows: [#" test with " quote"#])
assertEscapedCommand(#"" test with "" quote""#, [" test with quote"], windows: [#" test with " quote"#])
Copy link
Contributor

Choose a reason for hiding this comment

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

Identical?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, Unix has with quote, Windows has with " quote (double "" results in an escaped " on Windows and an empty String on Unix)

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.

Windows path separators in compile_commands.json not parsed correctly

3 participants