Skip to content

Commit 578f2a7

Browse files
rgoldbergrgoldberg
committed
Improve flag & option name quoting in completion generation for all 3 shells.
Improve zsh quoting. Many quoting issues remain. To fix them, there should first be enforced & documented limits on the acceptable characters in various values throughout SAP to avoid unnecessary quoting. Signed-off-by: Ross Goldberg <[email protected]>
1 parent fd0db17 commit 578f2a7

10 files changed

+103
-84
lines changed

‎Sources/ArgumentParser/Completions/BashCompletionsGenerator.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ extension [ParsableCommand.Type] {
236236
if arg.isNullary { return nil }
237237

238238
return """
239-
\(arg.bashCompletionWords.joined(separator: "|")))
239+
\(arg.bashCompletionWords.map { "'\($0.shellEscapeForSingleQuotedString())'" }.joined(separator: "|")))
240240
\(bashValueCompletion(arg).indentingEachLine(by: 8))\
241241
return
242242
;;

‎Sources/ArgumentParser/Completions/FishCompletionsGenerator.swift

+3-3
Original file line numberDiff line numberDiff line change
@@ -316,11 +316,11 @@ extension Name {
316316
fileprivate var asFishSuggestion: String {
317317
switch self {
318318
case .long(let longName):
319-
return "-l \(longName)"
319+
return "-l '\(longName.fishEscapeForSingleQuotedString())'"
320320
case .short(let shortName, _):
321-
return "-s \(shortName)"
321+
return "-s '\(String(shortName).fishEscapeForSingleQuotedString())'"
322322
case .longWithSingleDash(let dashedName):
323-
return "-o \(dashedName)"
323+
return "-o '\(dashedName.fishEscapeForSingleQuotedString())'"
324324
}
325325
}
326326
}

‎Sources/ArgumentParser/Completions/ZshCompletionsGenerator.swift

+33-19
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,13 @@ extension [ParsableCommand.Type] {
7777
local -ar subcommands=(
7878
\(
7979
subcommands.map { """
80-
'\($0._commandName):\($0.configuration.abstract.zshEscapeForSingleQuotedExplanation())'
80+
'\($0._commandName.zshEscapeForSingleQuotedDescribeCompletion()):\($0.configuration.abstract.shellEscapeForSingleQuotedString())'
8181
"""
8282
}
8383
.joined(separator: "\n")
8484
)
8585
)
86-
_describe "subcommand" subcommands
86+
_describe subcommand subcommands
8787
;;
8888
arg)
8989
case "${words[1]}" in
@@ -146,10 +146,12 @@ extension [ParsableCommand.Type] {
146146
line = ""
147147
case 1:
148148
line = """
149-
\(arg.isRepeatableOption ? "*" : "")\(arg.names[0].synopsisString)\(arg.zshCompletionAbstract)
149+
\(arg.isRepeatableOption ? "*" : "")\(arg.names[0].synopsisString.zshEscapeForSingleQuotedOptionSpec())\(arg.zshCompletionAbstract)
150150
"""
151151
default:
152-
let synopses = arg.names.map { $0.synopsisString }
152+
let synopses = arg.names.map {
153+
$0.synopsisString.zshEscapeForSingleQuotedOptionSpec()
154+
}
153155
line = """
154156
\(arg.isRepeatableOption ? "*" : "(\(synopses.joined(separator: " ")))")'\
155157
{\(synopses.joined(separator: ","))}\
@@ -160,7 +162,10 @@ extension [ParsableCommand.Type] {
160162
switch arg.update {
161163
case .unary:
162164
let (argumentAction, setupScript) = argumentActionAndSetupScript(arg)
163-
return ("'\(line):\(arg.valueName):\(argumentAction)'", setupScript)
165+
return (
166+
"'\(line):\(arg.valueName.zshEscapeForSingleQuotedOptionSpec()):\(argumentAction)'",
167+
setupScript
168+
)
164169
case .nullary:
165170
return ("'\(line)'", nil)
166171
}
@@ -179,7 +184,7 @@ extension [ParsableCommand.Type] {
179184
extensions.isEmpty
180185
? ("_files", nil)
181186
: (
182-
"_files -g '\\''\(extensions.map { "*.\($0.zshEscapeForSingleQuotedExplanation())" }.joined(separator: " "))'\\''",
187+
"_files -g '\\''\(extensions.map { "*.\($0.shellEscapeForSingleQuotedString())" }.joined(separator: " "))'\\''",
183188
nil
184189
)
185190

@@ -239,17 +244,6 @@ extension [ParsableCommand.Type] {
239244
}
240245
}
241246

242-
extension String {
243-
fileprivate func zshEscapeForSingleQuotedExplanation() -> String {
244-
replacingOccurrences(
245-
of: #"[\\\[\]]"#,
246-
with: #"\\$0"#,
247-
options: .regularExpression
248-
)
249-
.shellEscapeForSingleQuotedString()
250-
}
251-
}
252-
253247
extension ArgumentDefinition {
254248
/// - returns: `true` if `self` is an option and can be tab-completed multiple times in one command line.
255249
/// For example, `ssh` allows the `-L` option to be given multiple times, to establish multiple port forwardings.
@@ -266,7 +260,27 @@ extension ArgumentDefinition {
266260
}
267261

268262
fileprivate var zshCompletionAbstract: String {
269-
guard !help.abstract.isEmpty else { return "" }
270-
return "[\(help.abstract.zshEscapeForSingleQuotedExplanation())]"
263+
help.abstract.isEmpty
264+
? ""
265+
: "[\(help.abstract.zshEscapeForSingleQuotedOptionSpec())]"
266+
}
267+
}
268+
269+
extension String {
270+
fileprivate func zshEscapeForSingleQuotedDescribeCompletion() -> String {
271+
replacingOccurrences(
272+
of: #"[:\\]"#,
273+
with: #"\\$0"#,
274+
options: .regularExpression
275+
)
276+
.shellEscapeForSingleQuotedString()
277+
}
278+
fileprivate func zshEscapeForSingleQuotedOptionSpec() -> String {
279+
replacingOccurrences(
280+
of: #"[:\\\[\]]"#,
281+
with: #"\\$0"#,
282+
options: .regularExpression
283+
)
284+
.shellEscapeForSingleQuotedString()
271285
}
272286
}

‎Tests/ArgumentParserExampleTests/Snapshots/testMathBashCompletionScript().bash

+6-6
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ _math_stats_average() {
218218

219219
# Offer option value completions
220220
case "${prev}" in
221-
--kind)
221+
'--kind')
222222
__math_add_completions -W 'mean'$'\n''median'$'\n''mode'
223223
return
224224
;;
@@ -238,23 +238,23 @@ _math_stats_quantiles() {
238238

239239
# Offer option value completions
240240
case "${prev}" in
241-
--file)
241+
'--file')
242242
__math_add_completions -o plusdirs -fX '!*.@(txt|md)'
243243
return
244244
;;
245-
--directory)
245+
'--directory')
246246
__math_add_completions -d
247247
return
248248
;;
249-
--shell)
249+
'--shell')
250250
__math_add_completions -W "$(eval 'head -100 /usr/share/dict/words | tail -50')"
251251
return
252252
;;
253-
--custom)
253+
'--custom')
254254
__math_add_completions -W "$(__math_custom_complete ---completion stats quantiles -- --custom "${COMP_CWORD}" "$(__math_cursor_index_in_current_word)")"
255255
return
256256
;;
257-
--custom-deprecated)
257+
'--custom-deprecated')
258258
__math_add_completions -W "$(__math_custom_complete ---completion stats quantiles -- --custom-deprecated)"
259259
return
260260
;;

‎Tests/ArgumentParserExampleTests/Snapshots/testMathFishCompletionScript().fish

+23-23
Original file line numberDiff line numberDiff line change
@@ -78,36 +78,36 @@ function __math_custom_completion
7878
end
7979
8080
complete -c 'math' -f
81-
complete -c 'math' -n '__math_should_offer_completions_for "math"' -l version -d 'Show the version.'
82-
complete -c 'math' -n '__math_should_offer_completions_for "math"' -s h -l help -d 'Show help information.'
81+
complete -c 'math' -n '__math_should_offer_completions_for "math"' -l 'version' -d 'Show the version.'
82+
complete -c 'math' -n '__math_should_offer_completions_for "math"' -s 'h' -l 'help' -d 'Show help information.'
8383
complete -c 'math' -n '__math_should_offer_completions_for "math" 1' -fa 'add' -d 'Print the sum of the values.'
8484
complete -c 'math' -n '__math_should_offer_completions_for "math" 1' -fa 'multiply' -d 'Print the product of the values.'
8585
complete -c 'math' -n '__math_should_offer_completions_for "math" 1' -fa 'stats' -d 'Calculate descriptive statistics.'
8686
complete -c 'math' -n '__math_should_offer_completions_for "math" 1' -fa 'help' -d 'Show subcommand help information.'
87-
complete -c 'math' -n '__math_should_offer_completions_for "math add"' -l hex-output -s x -d 'Use hexadecimal notation for the result.'
88-
complete -c 'math' -n '__math_should_offer_completions_for "math add"' -l version -d 'Show the version.'
89-
complete -c 'math' -n '__math_should_offer_completions_for "math add"' -s h -l help -d 'Show help information.'
90-
complete -c 'math' -n '__math_should_offer_completions_for "math multiply"' -l hex-output -s x -d 'Use hexadecimal notation for the result.'
91-
complete -c 'math' -n '__math_should_offer_completions_for "math multiply"' -l version -d 'Show the version.'
92-
complete -c 'math' -n '__math_should_offer_completions_for "math multiply"' -s h -l help -d 'Show help information.'
93-
complete -c 'math' -n '__math_should_offer_completions_for "math stats"' -l version -d 'Show the version.'
94-
complete -c 'math' -n '__math_should_offer_completions_for "math stats"' -s h -l help -d 'Show help information.'
87+
complete -c 'math' -n '__math_should_offer_completions_for "math add"' -l 'hex-output' -s 'x' -d 'Use hexadecimal notation for the result.'
88+
complete -c 'math' -n '__math_should_offer_completions_for "math add"' -l 'version' -d 'Show the version.'
89+
complete -c 'math' -n '__math_should_offer_completions_for "math add"' -s 'h' -l 'help' -d 'Show help information.'
90+
complete -c 'math' -n '__math_should_offer_completions_for "math multiply"' -l 'hex-output' -s 'x' -d 'Use hexadecimal notation for the result.'
91+
complete -c 'math' -n '__math_should_offer_completions_for "math multiply"' -l 'version' -d 'Show the version.'
92+
complete -c 'math' -n '__math_should_offer_completions_for "math multiply"' -s 'h' -l 'help' -d 'Show help information.'
93+
complete -c 'math' -n '__math_should_offer_completions_for "math stats"' -l 'version' -d 'Show the version.'
94+
complete -c 'math' -n '__math_should_offer_completions_for "math stats"' -s 'h' -l 'help' -d 'Show help information.'
9595
complete -c 'math' -n '__math_should_offer_completions_for "math stats" 1' -fa 'average' -d 'Print the average of the values.'
9696
complete -c 'math' -n '__math_should_offer_completions_for "math stats" 1' -fa 'stdev' -d 'Print the standard deviation of the values.'
9797
complete -c 'math' -n '__math_should_offer_completions_for "math stats" 1' -fa 'quantiles' -d 'Print the quantiles of the values (TBD).'
98-
complete -c 'math' -n '__math_should_offer_completions_for "math stats average"' -l kind -d 'The kind of average to provide.' -rfka 'mean median mode'
99-
complete -c 'math' -n '__math_should_offer_completions_for "math stats average"' -l version -d 'Show the version.'
100-
complete -c 'math' -n '__math_should_offer_completions_for "math stats average"' -s h -l help -d 'Show help information.'
101-
complete -c 'math' -n '__math_should_offer_completions_for "math stats stdev"' -l version -d 'Show the version.'
102-
complete -c 'math' -n '__math_should_offer_completions_for "math stats stdev"' -s h -l help -d 'Show help information.'
98+
complete -c 'math' -n '__math_should_offer_completions_for "math stats average"' -l 'kind' -d 'The kind of average to provide.' -rfka 'mean median mode'
99+
complete -c 'math' -n '__math_should_offer_completions_for "math stats average"' -l 'version' -d 'Show the version.'
100+
complete -c 'math' -n '__math_should_offer_completions_for "math stats average"' -s 'h' -l 'help' -d 'Show help information.'
101+
complete -c 'math' -n '__math_should_offer_completions_for "math stats stdev"' -l 'version' -d 'Show the version.'
102+
complete -c 'math' -n '__math_should_offer_completions_for "math stats stdev"' -s 'h' -l 'help' -d 'Show help information.'
103103
complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles" 1' -fka 'alphabet alligator branch braggart'
104104
complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles" 2' -fka '(__math_custom_completion ---completion stats quantiles -- customArg (count (__math_tokens -pc)) (__math_tokens -tC))'
105105
complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles" 3' -fka '(__math_custom_completion ---completion stats quantiles -- customDeprecatedArg)'
106-
complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l file -rfa '(set -l exts \'txt\' \'md\';for p in (string match -e -- \'*/\' (commandline -t);or printf \n)*.{$exts};printf %s\n $p;end;__fish_complete_directories (commandline -t) \'\')'
107-
complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l directory -rfa '(__math_complete_directories)'
108-
complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l shell -rfka '(head -100 /usr/share/dict/words | tail -50)'
109-
complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l custom -rfka '(__math_custom_completion ---completion stats quantiles -- --custom (count (__math_tokens -pc)) (__math_tokens -tC))'
110-
complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l custom-deprecated -rfka '(__math_custom_completion ---completion stats quantiles -- --custom-deprecated)'
111-
complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l version -d 'Show the version.'
112-
complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -s h -l help -d 'Show help information.'
113-
complete -c 'math' -n '__math_should_offer_completions_for "math help"' -l version -d 'Show the version.'
106+
complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l 'file' -rfa '(set -l exts \'txt\' \'md\';for p in (string match -e -- \'*/\' (commandline -t);or printf \n)*.{$exts};printf %s\n $p;end;__fish_complete_directories (commandline -t) \'\')'
107+
complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l 'directory' -rfa '(__math_complete_directories)'
108+
complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l 'shell' -rfka '(head -100 /usr/share/dict/words | tail -50)'
109+
complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l 'custom' -rfka '(__math_custom_completion ---completion stats quantiles -- --custom (count (__math_tokens -pc)) (__math_tokens -tC))'
110+
complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l 'custom-deprecated' -rfka '(__math_custom_completion ---completion stats quantiles -- --custom-deprecated)'
111+
complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -l 'version' -d 'Show the version.'
112+
complete -c 'math' -n '__math_should_offer_completions_for "math stats quantiles"' -s 'h' -l 'help' -d 'Show help information.'
113+
complete -c 'math' -n '__math_should_offer_completions_for "math help"' -l 'version' -d 'Show the version.'

‎Tests/ArgumentParserExampleTests/Snapshots/testMathZshCompletionScript().zsh

+2-2
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ _math() {
5555
'stats:Calculate descriptive statistics.'
5656
'help:Show subcommand help information.'
5757
)
58-
_describe "subcommand" subcommands
58+
_describe subcommand subcommands
5959
;;
6060
arg)
6161
case "${words[1]}" in
@@ -111,7 +111,7 @@ _math_stats() {
111111
'stdev:Print the standard deviation of the values.'
112112
'quantiles:Print the quantiles of the values (TBD).'
113113
)
114-
_describe "subcommand" subcommands
114+
_describe subcommand subcommands
115115
;;
116116
arg)
117117
case "${words[1]}" in

‎Tests/ArgumentParserUnitTests/CompletionScriptTests.swift

+6-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,12 @@ extension CompletionScriptTests {
8888
}
8989

9090
struct EscapedCommand: ParsableCommand {
91-
@Option(help: #"Escaped chars: '[]\."#)
91+
@Option(
92+
name: .customLong("o:n[e"),
93+
help: ArgumentHelp(
94+
#"Escaped chars: '[]\."#, valueName: "path[:options]"
95+
)
96+
)
9297
var one: String
9398

9499
@Argument(completion: .custom { _, _, _ in candidates(prefix: "i") })

‎Tests/ArgumentParserUnitTests/Snapshots/testBase_Bash().bash

+10-10
Original file line numberDiff line numberDiff line change
@@ -164,33 +164,33 @@ _base_test() {
164164

165165
# Offer option value completions
166166
case "${prev}" in
167-
--name)
167+
'--name')
168168
return
169169
;;
170-
--kind)
170+
'--kind')
171171
__base_test_add_completions -W 'one'$'\n''two'$'\n''custom-three'
172172
return
173173
;;
174-
--other-kind)
174+
'--other-kind')
175175
__base_test_add_completions -W 'b1_bash'$'\n''b2_bash'$'\n''b3_bash'
176176
return
177177
;;
178-
--path1)
178+
'--path1')
179179
__base_test_add_completions -f
180180
return
181181
;;
182-
--path2)
182+
'--path2')
183183
__base_test_add_completions -f
184184
return
185185
;;
186-
--path3)
186+
'--path3')
187187
__base_test_add_completions -W 'c1_bash'$'\n''c2_bash'$'\n''c3_bash'
188188
return
189189
;;
190-
--rep1)
190+
'--rep1')
191191
return
192192
;;
193-
-r|--rep2)
193+
'-r'|'--rep2')
194194
return
195195
;;
196196
esac
@@ -231,12 +231,12 @@ _base_test_sub_command() {
231231

232232
_base_test_escaped_command() {
233233
flags=(-h --help)
234-
options=(--one)
234+
options=(--o:n[e)
235235
__base_test_offer_flags_options 1
236236

237237
# Offer option value completions
238238
case "${prev}" in
239-
--one)
239+
'--o:n[e')
240240
return
241241
;;
242242
esac

0 commit comments

Comments
 (0)