Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Honor spaces in autocomplete candidates #1759

Merged
merged 8 commits into from
Oct 31, 2022
Merged

Honor spaces in autocomplete candidates #1759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4bc3a48
Fixes #1754
jsotuyod Jul 28, 2022
568fc33
Rework to use a helper and support positional params
jsotuyod Jul 29, 2022
eb1a7c8
Update tests to pass
jsotuyod Jul 29, 2022
5c00bc0
Fix usage example
jsotuyod Jul 29, 2022
aed23ba
Merge branch 'main' into fix-1754
remkop Aug 18, 2022
37f5849
Merge branch 'main' into fix-1754
remkop Oct 31, 2022
10e97ff
Merge branch 'main' into fix-1754
remkop Oct 31, 2022
de32d72
Merge branch 'main' into fix-1754
remkop Oct 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 42 additions & 6 deletions src/main/java/picocli/AutoComplete.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,41 @@ private static class CommandDescriptor {
" done\n" +
" echo \"$result\"\n" +
"}\n" +
"\n" +
"# compReplyArray generates a list of completion suggestions based on an array, ensuring all values are properly escaped.\n" +
"#\n" +
"# compReplyArray takes a single parameter: the array of options to be displayed\n" +
"#\n" +
"# The output is echoed to std_out, one option per line.\n"
+ "#\n"
+ "# Example usage:\n"
+ "# local options=(\"foo\", \"bar\", \"baz\")\n"
+ "# local IFS=$'\\n'\n"
+ "# COMPREPLY=($(compReplyArray \"${options[@]}\"))\n" +
"function compReplyArray() {\n" +
" declare -a options\n" +
" options=(\"$@\")\n" +
" local curr_word=${COMP_WORDS[COMP_CWORD]}\n" +
" local i\n" +
" local quoted\n" +
" local optionList=()\n" +
"\n" +
" for (( i=0; i<${#options[@]}; i++ )); do\n" +
" # Double escape, since we want escaped values, but compgen -W expands the argument\n" +
" printf -v quoted %%q \"${options[i]}\"\n" +
" quoted=\\'${quoted//\\'/\\'\\\\\\'\\'}\\'\n" +
"\n" +
" optionList[i]=$quoted\n" +
" done\n" +
"\n" +
" # We also have to add another round of escaping to $curr_word.\n" +
" curr_word=${curr_word//\\\\/\\\\\\\\}\n" +
" curr_word=${curr_word//\\'/\\\\\\'}\n" +
"\n" +
" # Actually generate completions.\n" +
" local IFS=$'\\n'\n" +
" echo -e \"$(compgen -W \"${optionList[*]}\" -- \"$curr_word\")\"\n" +
"}\n" +
"\n";

private static final String SCRIPT_FOOTER = "" +
Expand Down Expand Up @@ -716,16 +751,16 @@ private static String generateFunctionForCommand(String functionName, String com

private static void generatePositionParamCompletionCandidates(StringBuilder buff, PositionalParamSpec f) {
String paramName = bashify(f.paramLabel());
buff.append(format(" local %s_pos_param_args=\"%s\" # %d-%d values\n",
buff.append(format(" local %s_pos_param_args=(\"%s\") # %d-%d values\n",
paramName,
concat(" ", extract(f.completionCandidates())).trim(),
concat("\" \"", extract(f.completionCandidates())).trim(),
f.index().min(), f.index().max()));
}

private static void generateCompletionCandidates(StringBuilder buff, OptionSpec f) {
buff.append(format(" local %s_option_args=\"%s\" # %s values\n",
buff.append(format(" local %s_option_args=(\"%s\") # %s values\n",
bashify(f.paramLabel()),
concat(" ", extract(f.completionCandidates())).trim(),
concat("\" \"", extract(f.completionCandidates())).trim(),
f.longestName()));
}
private static List<String> extract(Iterable<String> generator) {
Expand All @@ -750,7 +785,7 @@ private static String generatePositionalParamsCases(List<PositionalParamSpec> po
int max = param.index().max();
if (param.completionCandidates() != null) {
buff.append(format("%s %s (( currIndex >= %d && currIndex <= %d )); then\n", indent, ifOrElif, min, max));
buff.append(format("%s positionals=$( compgen -W \"$%s_pos_param_args\" -- \"%s\" )\n", indent, paramName, currWord));
buff.append(format("%s positionals=$( compReplyArray \"${%s_pos_param_args[@]}\" )\n", indent, paramName, currWord));
Copy link
Owner

Choose a reason for hiding this comment

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

We recently discovered that the format string has only 2 %s variables, but we specify 3 parameters (indent, paramName, currWord)...

This means that the last parameter (currWord) is not used.
Is that correct? Can the currWord parameter be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it can be removed. Sorry for missing that

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed in #2175

} else if (type.equals(File.class) || "java.nio.file.Path".equals(type.getName())) {
buff.append(format("%s %s (( currIndex >= %d && currIndex <= %d )); then\n", indent, ifOrElif, min, max));
buff.append(format("%s local IFS=$'\\n'\n", indent));
Expand Down Expand Up @@ -793,7 +828,8 @@ private static String generateOptionsCases(List<OptionSpec> argOptionFields, Str
}
if (option.completionCandidates() != null) {
buff.append(format("%s %s)\n", indent, concat("|", option.names()))); // " -u|--timeUnit)\n"
buff.append(format("%s COMPREPLY=( $( compgen -W \"${%s_option_args}\" -- \"%s\" ) )\n", indent, bashify(option.paramLabel()), currWord));
buff.append(format("%s local IFS=$'\\n'\n", indent));
buff.append(format("%s COMPREPLY=( $( compReplyArray \"${%s_option_args[@]}\" ) )\n", indent, bashify(option.paramLabel()), currWord));
Copy link
Owner

Choose a reason for hiding this comment

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

We recently discovered that the format string has only 2 %s variables, but we specify 3 parameters (indent, bashify(option.paramLabel()), currWord)...

This means that the last parameter (currWord) is not used.
Is that correct? Can the currWord parameter be removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed in #2175

buff.append(format("%s return $?\n", indent));
buff.append(format("%s ;;\n", indent));
} else if (type.equals(File.class) || "java.nio.file.Path".equals(type.getName())) {
Expand Down
199 changes: 172 additions & 27 deletions src/test/java/picocli/AutoCompleteTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,15 @@
*/
package picocli;

import org.hamcrest.MatcherAssert;
import org.junit.Rule;
import org.junit.Test;
import org.junit.contrib.java.lang.system.Assertion;
import org.junit.contrib.java.lang.system.ExpectedSystemExit;
import org.junit.contrib.java.lang.system.ProvideSystemProperty;
import org.junit.contrib.java.lang.system.RestoreSystemProperties;
import org.junit.contrib.java.lang.system.SystemErrRule;
import org.junit.contrib.java.lang.system.SystemOutRule;
import org.junit.rules.TestRule;
import picocli.CommandLine.Command;
import picocli.CommandLine.Model.CommandSpec;
import picocli.CommandLine.Model.OptionSpec;
import picocli.CommandLine.Model.PositionalParamSpec;
import picocli.CommandLine.Option;
import picocli.CommandLine.Parameters;
import static java.lang.String.format;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.io.BufferedReader;
import java.io.File;
Expand All @@ -53,11 +46,23 @@
import java.util.Scanner;
import java.util.concurrent.TimeUnit;

import static java.lang.String.format;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.*;
import org.hamcrest.MatcherAssert;
import org.junit.Rule;
import org.junit.Test;
import org.junit.contrib.java.lang.system.Assertion;
import org.junit.contrib.java.lang.system.ExpectedSystemExit;
import org.junit.contrib.java.lang.system.ProvideSystemProperty;
import org.junit.contrib.java.lang.system.RestoreSystemProperties;
import org.junit.contrib.java.lang.system.SystemErrRule;
import org.junit.contrib.java.lang.system.SystemOutRule;
import org.junit.rules.TestRule;

import picocli.CommandLine.Command;
import picocli.CommandLine.Model.CommandSpec;
import picocli.CommandLine.Model.OptionSpec;
import picocli.CommandLine.Model.PositionalParamSpec;
import picocli.CommandLine.Option;
import picocli.CommandLine.Parameters;

/**
* Tests the scripts generated by AutoComplete.
Expand Down Expand Up @@ -93,7 +98,7 @@ public void run() {
public void basic() throws Exception {
String script = AutoComplete.bash("basicExample", new CommandLine(new BasicExample()));
String expected = format(loadTextFromClasspath("/basic.bash"),
CommandLine.VERSION, spaced(TimeUnit.values()));
CommandLine.VERSION, concat("\" \"", TimeUnit.values()));
assertEquals(expected, script);
}

Expand Down Expand Up @@ -188,7 +193,7 @@ public void nestedSubcommands() throws Exception {
);
String script = AutoComplete.bash("picocompletion-demo", hierarchy);
String expected = format(loadTextFromClasspath("/picocompletion-demo_completion.bash"),
CommandLine.VERSION, spaced(TimeUnit.values()));
CommandLine.VERSION, concat("\" \"", TimeUnit.values()));
assertEquals(expected, script);
}

Expand All @@ -204,16 +209,16 @@ public void helpCommand() {
.addSubcommand(new CommandLine.HelpCommand());
String script = AutoComplete.bash("picocompletion-demo-help", hierarchy);
String expected = format(loadTextFromClasspath("/picocompletion-demo-help_completion.bash"),
CommandLine.VERSION, spaced(TimeUnit.values()));
CommandLine.VERSION, concat("\" \"", TimeUnit.values()));
assertEquals(expected, script);
}

private static String spaced(Object[] values) {
private static String concat(String infix, Object[] values) {
StringBuilder result = new StringBuilder();
for (Object value : values) {
result.append(value).append(' ');
result.append(value).append(infix);
}
return result.toString().substring(0, result.length() - 1);
return result.toString().substring(0, result.length() - infix.length());
}

static String loadTextFromClasspath(String path) {
Expand Down Expand Up @@ -751,6 +756,41 @@ private String expectedCompletionScriptForAutoCompleteApp() {
" echo \"$result\"\n" +
"}\n" +
"\n" +
"# compReplyArray generates a list of completion suggestions based on an array, ensuring all values are properly escaped.\n" +
"#\n" +
"# compReplyArray takes a single parameter: the array of options to be displayed\n" +
"#\n" +
"# The output is echoed to std_out, one option per line.\n" +
"#\n" +
"# Example usage:\n" +
"# local options=(\"foo\", \"bar\", \"baz\")\n" +
"# local IFS=$'\\n'\n" +
"# COMPREPLY=($(compReplyArray \"${options[@]}\"))\n" +
"function compReplyArray() {\n" +
" declare -a options\n" +
" options=(\"$@\")\n" +
" local curr_word=${COMP_WORDS[COMP_CWORD]}\n" +
" local i\n" +
" local quoted\n" +
" local optionList=()\n" +
"\n" +
" for (( i=0; i<${#options[@]}; i++ )); do\n" +
" # Double escape, since we want escaped values, but compgen -W expands the argument\n" +
" printf -v quoted %%q \"${options[i]}\"\n" +
" quoted=\\'${quoted//\\'/\\'\\\\\\'\\'}\\'\n" +
"\n" +
" optionList[i]=$quoted\n" +
" done\n" +
"\n" +
" # We also have to add another round of escaping to $curr_word.\n" +
" curr_word=${curr_word//\\\\/\\\\\\\\}\n" +
" curr_word=${curr_word//\\'/\\\\\\'}\n" +
"\n" +
" # Actually generate completions.\n" +
" local IFS=$'\\n'\n" +
" echo -e \"$(compgen -W \"${optionList[*]}\" -- \"$curr_word\")\"\n" +
"}\n" +
"\n" +
"# Bash completion entry point function.\n" +
"# _complete_picocli.AutoComplete finds which commands and subcommands have been specified\n" +
"# on the command line and delegates to the appropriate function\n" +
Expand Down Expand Up @@ -969,6 +1009,41 @@ private String expectedCompletionScriptForNonDefault() {
" echo \"$result\"\n" +
"}\n" +
"\n" +
"# compReplyArray generates a list of completion suggestions based on an array, ensuring all values are properly escaped.\n" +
"#\n" +
"# compReplyArray takes a single parameter: the array of options to be displayed\n" +
"#\n" +
"# The output is echoed to std_out, one option per line.\n" +
"#\n" +
"# Example usage:\n" +
"# local options=(\"foo\", \"bar\", \"baz\")\n" +
"# local IFS=$'\\n'\n" +
"# COMPREPLY=($(compReplyArray \"${options[@]}\"))\n" +
"function compReplyArray() {\n" +
" declare -a options\n" +
" options=(\"$@\")\n" +
" local curr_word=${COMP_WORDS[COMP_CWORD]}\n" +
" local i\n" +
" local quoted\n" +
" local optionList=()\n" +
"\n" +
" for (( i=0; i<${#options[@]}; i++ )); do\n" +
" # Double escape, since we want escaped values, but compgen -W expands the argument\n" +
" printf -v quoted %%q \"${options[i]}\"\n" +
" quoted=\\'${quoted//\\'/\\'\\\\\\'\\'}\\'\n" +
"\n" +
" optionList[i]=$quoted\n" +
" done\n" +
"\n" +
" # We also have to add another round of escaping to $curr_word.\n" +
" curr_word=${curr_word//\\\\/\\\\\\\\}\n" +
" curr_word=${curr_word//\\'/\\\\\\'}\n" +
"\n" +
" # Actually generate completions.\n" +
" local IFS=$'\\n'\n" +
" echo -e \"$(compgen -W \"${optionList[*]}\" -- \"$curr_word\")\"\n" +
"}\n" +
"\n" +
"# Bash completion entry point function.\n" +
"# _complete_nondefault finds which commands and subcommands have been specified\n" +
"# on the command line and delegates to the appropriate function\n" +
Expand Down Expand Up @@ -1535,6 +1610,41 @@ private String getCompletionScriptText(String cmdName) {
" echo \"$result\"\n" +
"}\n" +
"\n" +
"# compReplyArray generates a list of completion suggestions based on an array, ensuring all values are properly escaped.\n" +
"#\n" +
"# compReplyArray takes a single parameter: the array of options to be displayed\n" +
"#\n" +
"# The output is echoed to std_out, one option per line.\n" +
"#\n" +
"# Example usage:\n" +
"# local options=(\"foo\", \"bar\", \"baz\")\n" +
"# local IFS=$'\\n'\n" +
"# COMPREPLY=($(compReplyArray \"${options[@]}\"))\n" +
"function compReplyArray() {\n" +
" declare -a options\n" +
" options=(\"$@\")\n" +
" local curr_word=${COMP_WORDS[COMP_CWORD]}\n" +
" local i\n" +
" local quoted\n" +
" local optionList=()\n" +
"\n" +
" for (( i=0; i<${#options[@]}; i++ )); do\n" +
" # Double escape, since we want escaped values, but compgen -W expands the argument\n" +
" printf -v quoted %%q \"${options[i]}\"\n" +
" quoted=\\'${quoted//\\'/\\'\\\\\\'\\'}\\'\n" +
"\n" +
" optionList[i]=$quoted\n" +
" done\n" +
"\n" +
" # We also have to add another round of escaping to $curr_word.\n" +
" curr_word=${curr_word//\\\\/\\\\\\\\}\n" +
" curr_word=${curr_word//\\'/\\\\\\'}\n" +
"\n" +
" # Actually generate completions.\n" +
" local IFS=$'\\n'\n" +
" echo -e \"$(compgen -W \"${optionList[*]}\" -- \"$curr_word\")\"\n" +
"}\n" +
"\n" +
"# Bash completion entry point function.\n" +
"# _complete_%1$s finds which commands and subcommands have been specified\n" +
"# on the command line and delegates to the appropriate function\n" +
Expand Down Expand Up @@ -1747,6 +1857,41 @@ private String getCompletionScriptTextWithHidden(String commandName) {
" echo \"$result\"\n" +
"}\n" +
"\n" +
"# compReplyArray generates a list of completion suggestions based on an array, ensuring all values are properly escaped.\n" +
"#\n" +
"# compReplyArray takes a single parameter: the array of options to be displayed\n" +
"#\n" +
"# The output is echoed to std_out, one option per line.\n" +
"#\n" +
"# Example usage:\n" +
"# local options=(\"foo\", \"bar\", \"baz\")\n" +
"# local IFS=$'\\n'\n" +
"# COMPREPLY=($(compReplyArray \"${options[@]}\"))\n" +
"function compReplyArray() {\n" +
" declare -a options\n" +
" options=(\"$@\")\n" +
" local curr_word=${COMP_WORDS[COMP_CWORD]}\n" +
" local i\n" +
" local quoted\n" +
" local optionList=()\n" +
"\n" +
" for (( i=0; i<${#options[@]}; i++ )); do\n" +
" # Double escape, since we want escaped values, but compgen -W expands the argument\n" +
" printf -v quoted %%q \"${options[i]}\"\n" +
" quoted=\\'${quoted//\\'/\\'\\\\\\'\\'}\\'\n" +
"\n" +
" optionList[i]=$quoted\n" +
" done\n" +
"\n" +
" # We also have to add another round of escaping to $curr_word.\n" +
" curr_word=${curr_word//\\\\/\\\\\\\\}\n" +
" curr_word=${curr_word//\\'/\\\\\\'}\n" +
"\n" +
" # Actually generate completions.\n" +
" local IFS=$'\\n'\n" +
" echo -e \"$(compgen -W \"${optionList[*]}\" -- \"$curr_word\")\"\n" +
"}\n" +
"\n" +
"# Bash completion entry point function.\n" +
"# _complete_%1$s finds which commands and subcommands have been specified\n" +
"# on the command line and delegates to the appropriate function\n" +
Expand Down
Loading