From 94041974e4656da3401a0f728f59f5b774df48ed Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Mon, 24 Jul 2017 17:54:54 -0500 Subject: [PATCH] Added option to use completion source order without re-sorting Introduce a -k/--keep-order switch to `complete` that can be used to prevent fish from sorting/re-ordering the results provided by a completion source. In addition, this patch does so without doing away with deduplication of completions by introducing a new unique_unsorted(..) helper function that removes duplicates in-place without affecting the general order of the vector/container. Note that the code now uses a stable sort for completions, since the behavior of is_naturally_less_than as of this patch now means that the results are not necessarily _actually_ identical just because that function repeatedly returns false for any ordering of any given two elements. Fixes #361 --- CHANGELOG.md | 1 + doc_src/complete.txt | 3 +++ src/builtin_complete.cpp | 11 ++++++++++- src/complete.cpp | 31 ++++++++++++++++++++++--------- src/complete.h | 9 +++++++-- 5 files changed, 43 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06396c2a5..182996478 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ This section is for changes merged to the `major` branch that are not also merge - `read` now requires at least one var name (#4220). - Local exported (`set -lx`) vars are now visible to functions (#1091). - `set x[1] x[2] a b` is no longer valid syntax (#4236). +- `complete` now has a `-k`/`--keep` flag to keep the order of the OPTION_ARGUMENTS (#361). ## Other significant changes diff --git a/doc_src/complete.txt b/doc_src/complete.txt index 5a1a86fa7..d295d1381 100644 --- a/doc_src/complete.txt +++ b/doc_src/complete.txt @@ -8,6 +8,7 @@ complete ( -c | --command | -p | --path ) COMMAND [( -s | --short-option ) SHORT_OPTION]... [( -l | --long-option | -o | --old-option ) LONG_OPTION]... [( -a | --arguments ) OPTION_ARGUMENTS] + [( -k | --keep-order )] [( -f | --no-files )] [( -r | --require-parameter )] [( -x | --exclusive )] @@ -47,6 +48,8 @@ the fish manual. - `-a OPTION_ARGUMENTS` or `--arguments=OPTION_ARGUMENTS` adds the specified option arguments to the completions list. +- `-k` or `--keep-order` preserves the order of the `OPTION_ARGUMENTS` specified via `-a` or `--arguments` instead of sorting alphabetically. + - `-f` or `--no-files` specifies that the options specified by this completion may not be followed by a filename. - `-r` or `--require-parameter` specifies that the options specified by this completion always must have an option argument, i.e. may not be followed by another option. diff --git a/src/builtin_complete.cpp b/src/builtin_complete.cpp index c05c532fa..3bb36806e 100644 --- a/src/builtin_complete.cpp +++ b/src/builtin_complete.cpp @@ -128,8 +128,9 @@ int builtin_complete(parser_t &parser, io_streams_t &streams, wchar_t **argv) { wcstring_list_t cmd_to_complete; wcstring_list_t path; wcstring_list_t wrap_targets; + bool preserve_order = false; - static const wchar_t *short_options = L":a:c:p:s:l:o:d:frxeuAn:C::w:h"; + static const wchar_t *short_options = L":a:c:p:s:l:o:d:frxeuAn:C::w:hk"; static const struct woption long_options[] = {{L"exclusive", no_argument, NULL, 'x'}, {L"no-files", no_argument, NULL, 'f'}, {L"require-parameter", no_argument, NULL, 'r'}, @@ -147,6 +148,7 @@ int builtin_complete(parser_t &parser, io_streams_t &streams, wchar_t **argv) { {L"wraps", required_argument, NULL, 'w'}, {L"do-complete", optional_argument, NULL, 'C'}, {L"help", no_argument, NULL, 'h'}, + {L"keep-order", no_argument, NULL, 'k'}, {NULL, 0, NULL, 0}}; int opt; @@ -165,6 +167,10 @@ int builtin_complete(parser_t &parser, io_streams_t &streams, wchar_t **argv) { result_mode |= NO_COMMON; break; } + case 'k': { + preserve_order = true; + break; + } case 'p': case 'c': { wcstring tmp; @@ -355,6 +361,9 @@ int builtin_complete(parser_t &parser, io_streams_t &streams, wchar_t **argv) { streams.out.append(complete_print()); } else { int flags = COMPLETE_AUTO_SPACE; + if (preserve_order) { + flags |= COMPLETE_DONT_SORT; + } if (remove) { builtin_complete_remove(cmd_to_complete, path, short_opt.c_str(), gnu_opt, old_opt); diff --git a/src/complete.cpp b/src/complete.cpp index 3ca2c8131..d537b0bf3 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include "autoload.h" #include "builtin.h" @@ -224,13 +225,14 @@ completion_t &completion_t::operator=(const completion_t &him) { } bool completion_t::is_naturally_less_than(const completion_t &a, const completion_t &b) { + // For this to work, stable_sort must be used because results aren't interchangeable. + if (a.flags & b.flags & COMPLETE_DONT_SORT) { + // Both completions are from a source with the --keep-order flag. + return false; + } return wcsfilecmp(a.completion.c_str(), b.completion.c_str()) < 0; } -bool completion_t::is_alphabetically_equal_to(const completion_t &a, const completion_t &b) { - return a.completion == b.completion; -} - void completion_t::prepend_token_prefix(const wcstring &prefix) { if (this->flags & COMPLETE_REPLACES_TOKEN) { this->completion.insert(0, prefix); @@ -241,6 +243,16 @@ static bool compare_completions_by_match_type(const completion_t &a, const compl return a.match.type < b.match.type; } +template +static Iterator unique_unsorted(Iterator begin, Iterator end, HashFunction hash) { + typedef typename std::iterator_traits::value_type T; + + std::unordered_set temp; + return std::remove_if(begin, end, [&](const T& val) { + return !temp.insert(hash(val)).second; + }); +} + void completions_sort_and_prioritize(std::vector *comps) { // Find the best match type. fuzzy_match_type_t best_type = fuzzy_match_none; @@ -261,11 +273,12 @@ void completions_sort_and_prioritize(std::vector *comps) { } } - // Remove duplicates. - sort(comps->begin(), comps->end(), completion_t::is_naturally_less_than); - comps->erase( - std::unique(comps->begin(), comps->end(), completion_t::is_alphabetically_equal_to), - comps->end()); + // Sort, provided COMPLETION_DONT_SORT isn't set + stable_sort(comps->begin(), comps->end(), completion_t::is_naturally_less_than); + // Deduplicate both sorted and unsorted results + comps->erase(unique_unsorted(comps->begin(), comps->end(), [](const completion_t &c) { + return std::hash{}(c.completion); + }), comps->end()); // Sort the remainder by match type. They're already sorted alphabetically. stable_sort(comps->begin(), comps->end(), compare_completions_by_match_type); diff --git a/src/complete.h b/src/complete.h index afa3c7885..935dddd6a 100644 --- a/src/complete.h +++ b/src/complete.h @@ -41,7 +41,9 @@ enum { /// This completion should be inserted as-is, without escaping. COMPLETE_DONT_ESCAPE = 1 << 4, /// If you do escape, don't escape tildes. - COMPLETE_DONT_ESCAPE_TILDES = 1 << 5 + COMPLETE_DONT_ESCAPE_TILDES = 1 << 5, + /// Do not sort supplied completions + COMPLETE_DONT_SORT = 1 << 6 }; typedef int complete_flags_t; @@ -80,7 +82,10 @@ class completion_t { // "Naturally less than" means in a natural ordering, where digits are treated as numbers. For // example, foo10 is naturally greater than foo2 (but alphabetically less than it). static bool is_naturally_less_than(const completion_t &a, const completion_t &b); - static bool is_alphabetically_equal_to(const completion_t &a, const completion_t &b); + + // Deduplicate a potentially-unsorted vector, preserving the order + template + static Iterator unique_unsorted(Iterator begin, Iterator end, HashFunction hash); // If this completion replaces the entire token, prepend a prefix. Otherwise do nothing. void prepend_token_prefix(const wcstring &prefix);