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

@@ -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
 

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.

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);

src/complete.cpp

@@ -20,6 +20,7 @@
 #include <string>
 #include <type_traits>
 #include <utility>
+#include <unordered_set>
 
 #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 <class Iterator, class HashFunction>
+static Iterator unique_unsorted(Iterator begin, Iterator end, HashFunction hash) {
+    typedef typename std::iterator_traits<Iterator>::value_type T;
+
+    std::unordered_set<size_t> temp;
+    return std::remove_if(begin, end, [&](const T& val) {
+            return !temp.insert(hash(val)).second;
+            });
+}
+
 void completions_sort_and_prioritize(std::vector<completion_t> *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<completion_t> *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<wcstring>{}(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);

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 <class Iterator, class HashFunction>
+    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);