[pigeon] add support for multiple swift outputs - #12720
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for multiple output locations in the Swift generator via swiftOut and --swift_out. It updates option parsing, internal option models, and generation adapters to handle multiple paths, and adds corresponding unit tests. The review feedback suggests improving Windows compatibility by using platform-agnostic path.join instead of path.posix.join for file operations, and replacing backslashes with forward slashes before extracting file names from paths.
| if (outputPath == 'stdout') { | ||
| stdout.write(content); | ||
| } else { | ||
| final file = File(path.posix.join(options.basePath ?? '', outputPath)); |
There was a problem hiding this comment.
Using path.posix.join forces POSIX-style forward slashes as path separators. Since this code is performing local file system operations (File(...)), it is better to use platform-agnostic path.join to ensure correct path resolution on Windows and other platforms.
| final file = File(path.posix.join(options.basePath ?? '', outputPath)); | |
| final file = File(path.join(options.basePath ?? '', outputPath)); |
| (swiftOuts?.firstOrNull ?? swiftOut ?? '') | ||
| .split('/') | ||
| .lastOrNull | ||
| ?.split('.') | ||
| .firstOrNull ?? | ||
| '', |
There was a problem hiding this comment.
When running on Windows, file paths typically use backslashes (\\) as separators. Splitting only by / will fail to extract the file name correctly, leading to invalid or colliding class names (e.g., C:\\path\\to\\file.swift would result in C:\\path\\to\\file instead of file). Replacing backslashes with forward slashes before splitting resolves this issue.
| (swiftOuts?.firstOrNull ?? swiftOut ?? '') | |
| .split('/') | |
| .lastOrNull | |
| ?.split('.') | |
| .firstOrNull ?? | |
| '', | |
| (swiftOuts?.firstOrNull ?? swiftOut ?? '') | |
| .replaceAll('\\', '/') | |
| .split('/') | |
| .lastOrNull | |
| ?.split('.') | |
| .firstOrNull ?? | |
| '', |
1 participant
fixes flutter/flutter#164297