Fix generated shell completion script handling of repeating & non-repeating positional arguments, flags & options#808
Conversation
c819a12 to
1ae39ef
Compare
natecook1000
left a comment
There was a problem hiding this comment.
Still testing this locally, but looks good so far!
| \(declareTopLevelArray)repeating_options=(\(options.filter(\.isRepeating).flatMap(\.completionWords).joined(separator: " "))) | ||
| \(declareTopLevelArray)non_repeating_options=(\(options.filter { !$0.isRepeating }.flatMap(\.completionWords).joined(separator: " "))) | ||
| \(offerFlagsOptionsFunctionName) \ | ||
| \(positionalArguments.contains { $0.isRepeating } ? 9_223_372_036_854_775_807 : positionalArguments.count) |
There was a problem hiding this comment.
This Int.max literal doesn't seem platform-safe – can you say a bit about the role it's serving?
There was a problem hiding this comment.
If a positional arg is defined after a repeating positional arg, the prior repeating positional arg swallows all the positional arguments, so I wanted to ensure that no completion script is generated for any positional args after the first repeating positional arg.
I used the magic number to accept all non-flag/non-option/non-option-value arguments as positionals if there is any repeating positional.
I forgot that Swift runs on non-Macs…
Thanks for catching this.
There was a problem hiding this comment.
@natecook1000 Just pushed a fix for this. I also rewrote my earlier reply because it was wrong, as I misremembered exactly what the magic number was for; the solution was the same either way, but I realized I misspoke about its exact purpose when I went in to fix it. Early Monday isn't my best time for remembering stuff I haven't looked at for over a week… :)
ed5bcf3 to
eb86da3
Compare
|
@natecook1000 Thanks for the feedback. I've submitted fixes for the 2 issues you raised. Thanks again. |
|
@natecook1000 @rauhul Any way to merge this before 1.6.2? I can make whatever changes you might want. |
eb86da3 to
3829812
Compare
3829812 to
caaef07
Compare
Signed-off-by: Ross Goldberg <484615+rgoldberg@users.noreply.github.com>
…wift-format violation. swift-format seems to have a bug. Signed-off-by: Ross Goldberg <484615+rgoldberg@users.noreply.github.com>
Signed-off-by: Ross Goldberg <484615+rgoldberg@users.noreply.github.com>
Signed-off-by: Ross Goldberg <484615+rgoldberg@users.noreply.github.com>
Signed-off-by: Ross Goldberg <484615+rgoldberg@users.noreply.github.com>
Signed-off-by: Ross Goldberg <484615+rgoldberg@users.noreply.github.com>
Signed-off-by: Ross Goldberg <484615+rgoldberg@users.noreply.github.com>
…eneration. Signed-off-by: Ross Goldberg <484615+rgoldberg@users.noreply.github.com>
… accept all subsequent non-flag/non-option/non-option-value tokens as positionals. Signed-off-by: Ross Goldberg <484615+rgoldberg@users.noreply.github.com>
caaef07 to
612722b
Compare
Fix generated shell completion script handling of repeating & non-repeating positional arguments, flags & options.
Also fix an unrelated swift-format violation & DocC typos in
Flag.swift.Resolve #806
Checklist