Add native response file support#909
Conversation
bkhouri
force-pushed
the
t/main/gh846_native_response_file_support
branch
from
72be887 to
2aaaca8
Compare
|
|
||
| ### Quoted Arguments | ||
|
|
||
| Response files support quoted arguments for values containing spaces: |
There was a problem hiding this comment.
question: quoting brings up the question of how escaping works for things like spaces in unquoted text, and escaping the quotes themselves. If you intend on using shell conventions, then what about single quotes, or escaping scapes with \?
There was a problem hiding this comment.
#41 has some links to how this is supported by some other languages/packages - some of them let one control the support the files will support.
Since someone could be using this support to migrate a tool to Swift, they might need the quoted form while some other migration might need the shell escaping support.
Likewise for what the marker on the command line is to say a file is a params/response file. So it if the leading @ maybe should be configurable.
There was a problem hiding this comment.
The documents has been updated to provide additional information on the quotes arguments.
|
|
||
| ### Nested Response Files | ||
|
|
||
| Response files can reference other response files: |
There was a problem hiding this comment.
suggestion: Maybe don't add nesting unless there's a compelling workflow to do so. By having response files it alleviates the problem of command lines that are too long for the OS, improves versioning, and clarity. I'm not sure if nesting adds much beyond that, and brings more levels of indirection for the user to figure out where an argument came from.
There was a problem hiding this comment.
LLVM's response file parsing supports nesting. I know that swift-driver today uses its own parsing instead of swift-argument-parser, but if there's any desire in the future to move existing LLVM-based tools from C++ onto Swift using swift-argument-parser, we'd need nested response file support to keep parity.
IMO, we should support it unless there's a compelling reason not to, to avoid unnecessary limitations on what the library can do. It looks like it falls out naturally from the implementation.
There was a problem hiding this comment.
The change currently supports nested response file.
|
|
||
| ## Literal At Signs | ||
|
|
||
| To use a literal `@` character in an argument, escape it with `@@`: |
There was a problem hiding this comment.
praise: it is good to call this kind of escaping out like this.
|
IMO this sort of feature will need to come with some sort of source location feature too |
bkhouri
force-pushed
the
t/main/gh846_native_response_file_support
branch
11 times, most recently
from
34c2c7d to
c4a089d
Compare
Add native response file support in Swift Argument Parser for all `ParsableCommand` and `AsyncParsableCommand` commands. Since the nested response files are supported, also include an experiemental command line argument that provides source location support. Fixes: #846
bkhouri
force-pushed
the
t/main/gh846_native_response_file_support
branch
from
c4a089d to
5cf5af6
Compare
Thanks for this feedback. A source location feature has been added. |
|
marking as |
5 participants
Add native response file support in Swift Argument Parser for all
ParsableCommandandAsyncParsableCommandcommands.Fixes: #846
Checklist