From 1d9886d0f73250eae2a4b8e7e0a5793597d1c8a7 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Tue, 2 Sep 2014 14:49:17 -0700 Subject: [PATCH 1/6] Don't segfault when erasing short option completions When using `complete -s x -e`, the long option is unspecified, which makes it NULL. Comparing this to a `wcstring` segfaults. Fixes #1182. --- complete.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/complete.cpp b/complete.cpp index 71df3e983..8ede1eb22 100644 --- a/complete.cpp +++ b/complete.cpp @@ -630,7 +630,7 @@ bool completion_entry_t::remove_option(wchar_t short_opt, const wchar_t *long_op for (option_list_t::iterator iter = this->options.begin(); iter != this->options.end();) { complete_entry_opt_t &o = *iter; - if (short_opt==o.short_opt || long_opt == o.long_opt) + if (short_opt==o.short_opt || (long_opt && long_opt == o.long_opt)) { /* fwprintf( stderr, L"remove option -%lc --%ls\n", From aa7fe3b1322e5cc912d6b132b7738a6c46b24046 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Tue, 2 Sep 2014 14:51:40 -0700 Subject: [PATCH 2/6] Don't erase all long opts with `complete -e` When using `complete -c foo -l bar -e`, all long options for the command were being erased because it was also comparing the short option, which was 0. --- complete.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/complete.cpp b/complete.cpp index 8ede1eb22..3cdffab9e 100644 --- a/complete.cpp +++ b/complete.cpp @@ -630,7 +630,8 @@ bool completion_entry_t::remove_option(wchar_t short_opt, const wchar_t *long_op for (option_list_t::iterator iter = this->options.begin(); iter != this->options.end();) { complete_entry_opt_t &o = *iter; - if (short_opt==o.short_opt || (long_opt && long_opt == o.long_opt)) + if ((short_opt && short_opt == o.short_opt) || + (long_opt && long_opt == o.long_opt)) { /* fwprintf( stderr, L"remove option -%lc --%ls\n", From edd4f3d5adb6b532ebc0e6633915613b39d0dba5 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Tue, 2 Sep 2014 14:53:19 -0700 Subject: [PATCH 3/6] Don't erase old-style options with `complete -l foo -e` When erasing long option completions, distinguish between gnu-style and old-style options, just like we do when adding and printing completions. --- builtin_complete.cpp | 19 +++++++++++++------ complete.cpp | 13 +++++++------ complete.h | 3 ++- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/builtin_complete.cpp b/builtin_complete.cpp index f0eb6e1b1..e306c739c 100644 --- a/builtin_complete.cpp +++ b/builtin_complete.cpp @@ -173,14 +173,16 @@ static void builtin_complete_add(const wcstring_list_t &cmd, static void builtin_complete_remove3(const wchar_t *cmd, int cmd_type, wchar_t short_opt, - const wcstring_list_t &long_opt) + const wcstring_list_t &long_opt, + int long_mode) { for (size_t i=0; iremove_option(short_opt, long_opt); + bool delete_it = entry->remove_option(short_opt, long_opt, old_mode); if (delete_it) { /* Delete this entry */ diff --git a/complete.h b/complete.h index 45eb2dc02..b5ad98c5e 100644 --- a/complete.h +++ b/complete.h @@ -207,7 +207,8 @@ void complete_set_authoritative(const wchar_t *cmd, bool cmd_type, bool authorit void complete_remove(const wchar_t *cmd, bool cmd_is_path, wchar_t short_opt, - const wchar_t *long_opt); + const wchar_t *long_opt, + int long_mode); /** Find all completions of the command cmd, insert them into out. From 2820c7a9cd1b9907b1a4dba0163d6d8bcb2c73eb Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Tue, 2 Sep 2014 15:10:52 -0700 Subject: [PATCH 4/6] Erase all completions with `complete -c foo -e` When passing `-e` to `complete -c foo` without any other options, all options for the command should be erased. Fixes #380. --- builtin_complete.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/builtin_complete.cpp b/builtin_complete.cpp index e306c739c..691fad2b7 100644 --- a/builtin_complete.cpp +++ b/builtin_complete.cpp @@ -224,6 +224,14 @@ static void builtin_complete_remove2(const wchar_t *cmd, } } } + else if (gnu_opt.empty() && old_opt.empty()) + { + complete_remove(cmd, + cmd_type, + 0, + 0, + 0); + } else { builtin_complete_remove3(cmd, From 90a4fd34d23e76b7934b040db3628d7884040267 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Tue, 2 Sep 2014 15:25:45 -0700 Subject: [PATCH 5/6] Add tests for the various `complete -e` changes --- tests/test6.in | 32 ++++++++++++++++++++++++++++++-- tests/test6.out | 30 +++++++++++++++++++++++++++++- 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/tests/test6.in b/tests/test6.in index e9b6e9b4d..69f42d56c 100755 --- a/tests/test6.in +++ b/tests/test6.in @@ -3,9 +3,37 @@ # We actually encountered some case that was effectively like this (Issue 2 in github) complete --command AAAA -l abcd --condition 'complete -c AAAA -l efgh' -complete -C'AAAA -' -complete -C'AAAA -' +echo "AAAA:" +complete -C'AAAA -' | sort +echo "AAAA:" +complete -C'AAAA -' | sort complete --command BBBB -l abcd --condition 'complete -e --command BBBB -l abcd' +echo "BBBB:" complete -C'BBBB -' +echo "BBBB:" complete -C'BBBB -' + +# Test that erasing completions works correctly +echo + +complete -c CCCC -l bar +complete -c CCCC -l baz +complete -c CCCC -o bar +complete -c CCCC -o foo +complete -c CCCC -s a +complete -c CCCC -s b +echo "CCCC:" +complete -C'CCCC -' | sort +complete -c CCCC -l bar -e +echo "CCCC:" +complete -C'CCCC -' | sort +complete -c CCCC -o foo -e +echo "CCCC:" +complete -C'CCCC -' | sort +complete -c CCCC -s a -e +echo "CCCC:" +complete -C'CCCC -' | sort +complete -c CCCC -e +echo "CCCC:" +complete -C'CCCC -' | sort diff --git a/tests/test6.out b/tests/test6.out index 0b248b03b..d61d3c5f5 100644 --- a/tests/test6.out +++ b/tests/test6.out @@ -1,4 +1,32 @@ +AAAA: +--abcd +AAAA: --abcd --efgh +BBBB: --abcd ---abcd +BBBB: + +CCCC: +--bar +--baz +-a +-b +-bar +-foo +CCCC: +--baz +-a +-b +-bar +-foo +CCCC: +--baz +-a +-b +-bar +CCCC: +--baz +-b +-bar +CCCC: From 1c4223889bd729ee83aa21a3450dc28f92ade641 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Tue, 2 Sep 2014 15:52:56 -0700 Subject: [PATCH 6/6] Fix test output for `complete -e` tests GNU sort behaves stupidly when LC_ALL is not C. This caused the test output to be sorted wrong. --- tests/test.fish | 4 ++-- tests/test6.in | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/test.fish b/tests/test.fish index 7573b4001..2f2b8533b 100755 --- a/tests/test.fish +++ b/tests/test.fish @@ -61,14 +61,14 @@ for i in *.in else set res fail echo Output differs for file $i. Diff follows: - diff tmp.out $template_out + diff -u tmp.out $template_out end if diff tmp.err $template_err >/dev/null else set res fail echo Error output differs for file $i. Diff follows: - diff tmp.err $template_err + diff -u tmp.err $template_err end if test (cat tmp.status) = (cat $template_status) diff --git a/tests/test6.in b/tests/test6.in index 69f42d56c..93be594af 100755 --- a/tests/test6.in +++ b/tests/test6.in @@ -17,6 +17,12 @@ complete -C'BBBB -' # Test that erasing completions works correctly echo +function sort + # GNU sort is really stupid, a non-C locale seems to make it assume --dictionary-order + # If I wanted --dictionary-order, I would have specified --dictionary-order! + env LC_ALL=C sort $argv +end + complete -c CCCC -l bar complete -c CCCC -l baz complete -c CCCC -o bar