Feature flag support for ? wildcard

This partially reverts 6e56637cf0 and #4520
by bringing back the ? wildcard, guarded by the qmark-noglob feature flag.

doc_src/index.hdr.in

@@ -92,6 +92,7 @@ Some characters can not be written directly on the command line. For these chara
 - '<code>\\$</code>' escapes the dollar character
 - '<code>\\\\</code>' escapes the backslash character
 - '<code>\\*</code>' escapes the star character
+- '<code>\\?</code>' escapes the question mark character
 - '<code>\\~</code>' escapes the tilde character
 - '<code>\\#</code>' escapes the hash character
 - '<code>\\(</code>' escapes the left parenthesis character
@@ -329,7 +330,7 @@ These are the general purpose tab completions that `fish` provides:
 
 - Completion of usernames for tilde expansion.
 
-- Completion of filenames, even on strings with wildcards such as '`*`' and '`**`'.
+- Completion of filenames, even on strings with wildcards such as '`*`', '`**`' and '`?`'.
 
 `fish` provides a large number of program specific completions. Most of these completions are simple options like the `-l` option for `ls`, but some are more advanced. The latter include:
 
@@ -417,7 +418,9 @@ When an argument for a program is given on the commandline, it undergoes the pro
 
 \subsection expand-wildcard Wildcards
 
-If a star (`*`) is present in the parameter, `fish` attempts to match the given parameter to any files in such a way that:
+If a star (`*`) or a question mark (`?`) is present in the parameter, `fish` attempts to match the given parameter to any files in such a way that:
+
+- `?` can match any single character except '/'.
 
 - `*` can match any string of characters not containing '/'. This includes matching an empty string.
 
@@ -443,6 +446,8 @@ Examples:
 
 - `a*` matches any files beginning with an 'a' in the current directory.
 
+- `???` matches any file in the current directory whose name is exactly three characters long.
+
 - `**` matches any files and directories in the current directory and all of its subdirectories.
 
 Note that for most commands, if any wildcard fails to expand, the command is not executed, <a href='#variables-status'>`$status`</a> is set to nonzero, and a warning is printed. This behavior is consistent with setting `shopt -s failglob` in bash. There are exactly 3 exceptions, namely <a href="commands.html#set">`set`</a>, <a href="commands.html#count">`count`</a> and <a href="commands.html#for">`for`</a>. Their globs are permitted to expand to zero arguments, as with `shopt -s nullglob` in bash.

share/completions/git.fish

@@ -127,7 +127,11 @@ function __fish_git_files
         # Be careful about the ordering here!
         #
         # HACK: To allow this to work both with and without '?' globs
-        set -l dq '??'
+        set -l dq '\\?\\?'
+        if status test-feature qmark-noglob
+            # ? is not a glob
+            set dq '??'
+        end
         switch "$stat"
             case DD AU UD UA DU AA UU
                 # Unmerged

src/common.cpp

@@ -933,6 +933,7 @@ static void escape_string_script(const wchar_t *orig_in, size_t in_len, wcstring
     const bool no_quoted = static_cast<bool>(flags & ESCAPE_NO_QUOTED);
     const bool no_tilde = static_cast<bool>(flags & ESCAPE_NO_TILDE);
     const bool no_caret = fish_features().test(features_t::stderr_nocaret);
+    const bool no_qmark = fish_features().test(features_t::qmark_noglob);
 
     int need_escape = 0;
     int need_complex_escape = 0;
@@ -997,6 +998,11 @@ static void escape_string_script(const wchar_t *orig_in, size_t in_len, wcstring
                     out += *in;
                     break;
                 }
+                case ANY_CHAR: {
+                    // See #1614
+                    out += L'?';
+                    break;
+                }
                 case ANY_STRING: {
                     out += L'*';
                     break;
@@ -1019,12 +1025,13 @@ static void escape_string_script(const wchar_t *orig_in, size_t in_len, wcstring
                 case L']':
                 case L'{':
                 case L'}':
+                case L'?':
                 case L'*':
                 case L'|':
                 case L';':
                 case L'"':
                 case L'~': {
-                    bool char_is_normal = (c == L'~' && no_tilde) || (c == L'^' && no_caret);
+                    bool char_is_normal = (c == L'~' && no_tilde) || (c == L'^' && no_caret) || (c == L'?' && no_qmark);
                     if (!char_is_normal) {
                         need_escape = 1;
                         if (escape_all) out += L'\\';
@@ -1353,6 +1360,12 @@ static bool unescape_string_internal(const wchar_t *const input, const size_t in
                     }
                     break;
                 }
+                case L'?': {
+                    if (unescape_special && !fish_features().test(features_t::qmark_noglob)) {
+                        to_append_or_none = ANY_CHAR;
+                    }
+                    break;
+                }
                 case L'$': {
                     if (unescape_special) {
                         to_append_or_none = VARIABLE_EXPAND;

src/expand.cpp

@@ -828,6 +828,10 @@ static void remove_internal_separator(wcstring *str, bool conv) {
     if (conv) {
         for (size_t idx = 0; idx < str->size(); idx++) {
             switch (str->at(idx)) {
+                case ANY_CHAR: {
+                    str->at(idx) = L'?';
+                    break;
+                }
                 case ANY_STRING:
                 case ANY_STRING_RECURSIVE: {
                     str->at(idx) = L'*';

src/fish_tests.cpp

@@ -4093,7 +4093,7 @@ static void test_wcstring_tok() {
 }
 
 int builtin_string(parser_t &parser, io_streams_t &streams, wchar_t **argv);
-static void run_one_string_test(const wchar_t **argv, int expected_rc,
+static void run_one_string_test(const wchar_t *const *argv, int expected_rc,
                                 const wchar_t *expected_out) {
     parser_t parser;
     io_streams_t streams(0);
@@ -4115,7 +4115,7 @@ static void run_one_string_test(const wchar_t **argv, int expected_rc,
 }
 
 static void test_string() {
-    static struct string_test {
+    const struct string_test {
         const wchar_t *argv[15];
         int expected_rc;
         const wchar_t *expected_out;
@@ -4159,16 +4159,15 @@ static void test_string() {
         {{L"string", L"match", 0}, STATUS_INVALID_ARGS, L""},
         {{L"string", L"match", L"", 0}, STATUS_CMD_ERROR, L""},
         {{L"string", L"match", L"", L"", 0}, STATUS_CMD_OK, L"\n"},
-        {{L"string", L"match", L"?", L"a", 0}, STATUS_CMD_ERROR, L""},
+        {{L"string", L"match", L"?", L"a", 0}, STATUS_CMD_OK, L"a\n"},
         {{L"string", L"match", L"*", L"", 0}, STATUS_CMD_OK, L"\n"},
         {{L"string", L"match", L"**", L"", 0}, STATUS_CMD_OK, L"\n"},
         {{L"string", L"match", L"*", L"xyzzy", 0}, STATUS_CMD_OK, L"xyzzy\n"},
         {{L"string", L"match", L"**", L"plugh", 0}, STATUS_CMD_OK, L"plugh\n"},
         {{L"string", L"match", L"a*b", L"axxb", 0}, STATUS_CMD_OK, L"axxb\n"},
-        {{L"string", L"match", L"a??b", L"axxb", 0}, STATUS_CMD_ERROR, L""},
+        {{L"string", L"match", L"a??b", L"axxb", 0}, STATUS_CMD_OK, L"axxb\n"},
-        {{L"string", L"match", L"a??b", L"a??b", 0}, STATUS_CMD_OK, L"a??b\n"},
+        {{L"string", L"match", L"-i", L"a??B", L"axxb", 0}, STATUS_CMD_OK, L"axxb\n"},
-        {{L"string", L"match", L"-i", L"a??B", L"axxb", 0}, STATUS_CMD_ERROR, L""},
+        {{L"string", L"match", L"-i", L"a??b", L"Axxb", 0}, STATUS_CMD_OK, L"Axxb\n"},
-        {{L"string", L"match", L"-i", L"a??b", L"Axxb", 0}, STATUS_CMD_ERROR, L""},
         {{L"string", L"match", L"a*", L"axxb", 0}, STATUS_CMD_OK, L"axxb\n"},
         {{L"string", L"match", L"*a", L"xxa", 0}, STATUS_CMD_OK, L"xxa\n"},
         {{L"string", L"match", L"*a*", L"axa", 0}, STATUS_CMD_OK, L"axa\n"},
@@ -4177,14 +4176,9 @@ static void test_string() {
         {{L"string", L"match", L"*a", L"a", 0}, STATUS_CMD_OK, L"a\n"},
         {{L"string", L"match", L"a*", L"a", 0}, STATUS_CMD_OK, L"a\n"},
         {{L"string", L"match", L"a*b*c", L"axxbyyc", 0}, STATUS_CMD_OK, L"axxbyyc\n"},
-        {{L"string", L"match", L"a*b?c", L"axxb?c", 0}, STATUS_CMD_OK, L"axxb?c\n"},
-        {{L"string", L"match", L"*?", L"a", 0}, STATUS_CMD_ERROR, L""},
-        {{L"string", L"match", L"*?", L"ab", 0}, STATUS_CMD_ERROR, L""},
-        {{L"string", L"match", L"?*", L"a", 0}, STATUS_CMD_ERROR, L""},
-        {{L"string", L"match", L"?*", L"ab", 0}, STATUS_CMD_ERROR, L""},
         {{L"string", L"match", L"\\*", L"*", 0}, STATUS_CMD_OK, L"*\n"},
         {{L"string", L"match", L"a*\\", L"abc\\", 0}, STATUS_CMD_OK, L"abc\\\n"},
-        {{L"string", L"match", L"a*\\?", L"abc?", 0}, STATUS_CMD_ERROR, L""},
+        {{L"string", L"match", L"a*\\?", L"abc?", 0}, STATUS_CMD_OK, L"abc?\n"},
 
         {{L"string", L"match", L"?", L"", 0}, STATUS_CMD_ERROR, L""},
         {{L"string", L"match", L"?", L"ab", 0}, STATUS_CMD_ERROR, L""},
@@ -4403,15 +4397,36 @@ static void test_string() {
         {{L"string", L"trim", L"-c", L"\\/", L"/a\\"}, STATUS_CMD_OK, L"a\n"},
         {{L"string", L"trim", L"-c", L"\\/", L"a/"}, STATUS_CMD_OK, L"a\n"},
         {{L"string", L"trim", L"-c", L"\\/", L"\\a/"}, STATUS_CMD_OK, L"a\n"},
-        {{L"string", L"trim", L"-c", L"", L".a."}, STATUS_CMD_ERROR, L".a.\n"},
+        {{L"string", L"trim", L"-c", L"", L".a."}, STATUS_CMD_ERROR, L".a.\n"}};
-
+    for (const auto &t : string_tests) {
-        {{NULL}, STATUS_CMD_ERROR, NULL}};
+        run_one_string_test(t.argv, t.expected_rc, t.expected_out);
-
-    struct string_test *t = string_tests;
-    while (t->argv[0]) {
-        run_one_string_test(t->argv, t->expected_rc, t->expected_out);
-        t++;
     }
+
+    const auto saved_flags = fish_features();
+    const struct string_test qmark_noglob_tests[] = {
+        {{L"string", L"match", L"a*b?c", L"axxb?c", 0}, STATUS_CMD_OK, L"axxb?c\n"},
+        {{L"string", L"match", L"*?", L"a", 0}, STATUS_CMD_ERROR, L""},
+        {{L"string", L"match", L"*?", L"ab", 0}, STATUS_CMD_ERROR, L""},
+        {{L"string", L"match", L"?*", L"a", 0}, STATUS_CMD_ERROR, L""},
+        {{L"string", L"match", L"?*", L"ab", 0}, STATUS_CMD_ERROR, L""},
+        {{L"string", L"match", L"a*\\?", L"abc?", 0}, STATUS_CMD_ERROR, L""}};
+    mutable_fish_features().set(features_t::qmark_noglob, true);
+    for (const auto &t : qmark_noglob_tests) {
+        run_one_string_test(t.argv, t.expected_rc, t.expected_out);
+    }
+
+    const struct string_test qmark_glob_tests[] = {
+        {{L"string", L"match", L"a*b?c", L"axxbyc", 0}, STATUS_CMD_OK, L"axxbyc\n"},
+        {{L"string", L"match", L"*?", L"a", 0}, STATUS_CMD_OK, L"a\n"},
+        {{L"string", L"match", L"*?", L"ab", 0}, STATUS_CMD_OK, L"ab\n"},
+        {{L"string", L"match", L"?*", L"a", 0}, STATUS_CMD_OK, L"a\n"},
+        {{L"string", L"match", L"?*", L"ab", 0}, STATUS_CMD_OK, L"ab\n"},
+        {{L"string", L"match", L"a*\\?", L"abc?", 0}, STATUS_CMD_OK, L"abc?\n"}};
+    mutable_fish_features().set(features_t::qmark_noglob, false);
+    for (const auto &t : qmark_glob_tests) {
+        run_one_string_test(t.argv, t.expected_rc, t.expected_out);
+    }
+    mutable_fish_features() = saved_flags;
 }
 
 /// Helper for test_timezone_env_vars().
@@ -4468,15 +4483,11 @@ static void test_illegal_command_exit_code() {
     };
 
     const command_result_tuple_t tests[] = {
-        {L"echo -n", STATUS_CMD_OK},
+        {L"echo -n", STATUS_CMD_OK}, {L"pwd", STATUS_CMD_OK},
-        {L"pwd", STATUS_CMD_OK},
+        // a `)` without a matching `(` is now a tokenizer error, and cannot be executed even as an illegal command
-        // a `)` without a matching `(` is now a tokenizer error, and cannot be executed even as an
-        // illegal command
         // {L")", STATUS_ILLEGAL_CMD},  {L") ", STATUS_ILLEGAL_CMD}, {L") ", STATUS_ILLEGAL_CMD}
-        {L"*", STATUS_ILLEGAL_CMD},
+        {L"*", STATUS_ILLEGAL_CMD},  {L"**", STATUS_ILLEGAL_CMD},
-        {L"**", STATUS_ILLEGAL_CMD},
+        {L"?", STATUS_ILLEGAL_CMD},  {L"abc?def", STATUS_ILLEGAL_CMD},
-        {L"?", STATUS_CMD_UNKNOWN},
-        {L"abc?def", STATUS_CMD_UNKNOWN},
     };
 
     int res = 0;

src/highlight.cpp

@@ -23,6 +23,7 @@
 #include "expand.h"
 #include "fallback.h"  // IWYU pragma: keep
 #include "function.h"
+#include "future_feature_flags.h"
 #include "highlight.h"
 #include "history.h"
 #include "output.h"
@@ -125,6 +126,7 @@ bool is_potential_path(const wcstring &potential_path_fragment, const wcstring_l
             case BRACE_BEGIN:
             case BRACE_END:
             case BRACE_SEP:
+            case ANY_CHAR:
             case ANY_STRING:
             case ANY_STRING_RECURSIVE: {
                 has_magic = 1;
@@ -547,6 +549,12 @@ static void color_argument_internal(const wcstring &buffstr,
                             in_pos -= 1;
                             break;
                         }
+                        case L'?': {
+                            if (!fish_features().test(features_t::qmark_noglob)) {
+                                colors[in_pos] = highlight_spec_operator;
+                            }
+                            break;
+                        }
                         case L'*':
                         case L'(':
                         case L')': {

src/parse_util.cpp

@@ -16,6 +16,7 @@
 #include "common.h"
 #include "expand.h"
 #include "fallback.h"  // IWYU pragma: keep
+#include "future_feature_flags.h"
 #include "parse_constants.h"
 #include "parse_util.h"
 #include "tnode.h"
@@ -418,14 +419,20 @@ void parse_util_token_extent(const wchar_t *buff, size_t cursor_pos, const wchar
 wcstring parse_util_unescape_wildcards(const wcstring &str) {
     wcstring result;
     result.reserve(str.size());
+    bool unesc_qmark = !fish_features().test(features_t::qmark_noglob);
 
     const wchar_t *const cs = str.c_str();
     for (size_t i = 0; cs[i] != L'\0'; i++) {
         if (cs[i] == L'*') {
             result.push_back(ANY_STRING);
+        } else if (cs[i] == L'?' && unesc_qmark) {
+            result.push_back(ANY_CHAR);
         } else if (cs[i] == L'\\' && cs[i + 1] == L'*') {
             result.push_back(cs[i + 1]);
             i += 1;
+        } else if (cs[i] == L'\\' && cs[i + 1] == L'?' && unesc_qmark) {
+            result.push_back(cs[i + 1]);
+            i += 1;
         } else if (cs[i] == L'\\' && cs[i + 1] == L'\\') {
             // Not a wildcard, but ensure the next iteration doesn't see this escaped backslash.
             result.append(L"\\\\");
@@ -890,7 +897,9 @@ void parse_util_expand_variable_error(const wcstring &token, size_t global_token
         default: {
             wchar_t token_stop_char = char_after_dollar;
             // Unescape (see issue #50).
-            if (token_stop_char == ANY_STRING || token_stop_char == ANY_STRING_RECURSIVE)
+            if (token_stop_char == ANY_CHAR)
+                token_stop_char = L'?';
+            else if (token_stop_char == ANY_STRING || token_stop_char == ANY_STRING_RECURSIVE)
                 token_stop_char = L'*';
 
             // Determine which error message to use. The format string may not consume all the

src/wildcard.cpp

@@ -19,6 +19,7 @@
 #include "complete.h"
 #include "expand.h"
 #include "fallback.h"  // IWYU pragma: keep
+#include "future_feature_flags.h"
 #include "reader.h"
 #include "wildcard.h"
 #include "wutil.h"  // IWYU pragma: keep
@@ -51,7 +52,7 @@
 /// Finds an internal (ANY_STRING, etc.) style wildcard, or wcstring::npos.
 static size_t wildcard_find(const wchar_t *wc) {
     for (size_t i = 0; wc[i] != L'\0'; i++) {
-        if (wc[i] == ANY_STRING || wc[i] == ANY_STRING_RECURSIVE) {
+        if (wc[i] == ANY_CHAR || wc[i] == ANY_STRING || wc[i] == ANY_STRING_RECURSIVE) {
             return i;
         }
     }
@@ -61,15 +62,17 @@ static size_t wildcard_find(const wchar_t *wc) {
 /// Implementation of wildcard_has. Needs to take the length to handle embedded nulls (issue #1631).
 static bool wildcard_has_impl(const wchar_t *str, size_t len, bool internal) {
     assert(str != NULL);
+    bool qmark_is_wild = !fish_features().test(features_t::qmark_noglob);
     const wchar_t *end = str + len;
     if (internal) {
         for (; str < end; str++) {
-            if (*str == ANY_STRING || *str == ANY_STRING_RECURSIVE) return true;
+            if ((*str == ANY_CHAR) || (*str == ANY_STRING) || (*str == ANY_STRING_RECURSIVE))
+                return true;
         }
     } else {
         wchar_t prev = 0;
         for (; str < end; str++) {
-            if (*str == L'*' && prev != L'\\') return true;
+            if (((*str == L'*') || (*str == L'?' && qmark_is_wild)) && (prev != L'\\')) return true;
             prev = *str;
         }
     }
@@ -127,6 +130,13 @@ static enum fuzzy_match_type_t wildcard_match_internal(const wchar_t *str, const
                 restart_is_out_of_str = (*str_x == 0);
                 wc_x++;
                 continue;
+            } else if (*wc_x == ANY_CHAR && *str_x != 0) {
+                if (is_first && *str_x == L'.') {
+                    return fuzzy_match_none;
+                }
+                wc_x++;
+                str_x++;
+                continue;
             } else if (*str_x != 0 && *str_x == *wc_x) { // ordinary character
                 wc_x++;
                 str_x++;
@@ -204,7 +214,7 @@ static bool wildcard_complete_internal(const wchar_t *str, const wchar_t *wc,
         return false;
     }
 
-    // Locate the next wildcard character position, e.g. ANY_STRING.
+    // Locate the next wildcard character position, e.g. ANY_CHAR or ANY_STRING.
     const size_t next_wc_char_pos = wildcard_find(wc);
 
     // Maybe we have no more wildcards at all. This includes the empty string.
@@ -257,6 +267,12 @@ static bool wildcard_complete_internal(const wchar_t *str, const wchar_t *wc,
     // Our first character is a wildcard.
     assert(next_wc_char_pos == 0);
     switch (wc[0]) {
+        case ANY_CHAR: {
+            if (str[0] == L'\0') {
+                return false;
+            }
+            return wildcard_complete_internal(str + 1, wc + 1, params, flags, out);
+        }
         case ANY_STRING: {
             // Hackish. If this is the last character of the wildcard, then just complete with
             // the empty string. This fixes cases like "f*<tab>" -> "f*o".
@@ -779,7 +795,7 @@ void wildcard_expander_t::expand_last_segment(const wcstring &base_dir, DIR *bas
 ///
 /// Args:
 /// base_dir: the "working directory" against which the wildcard is to be resolved
-/// wc: the wildcard string itself, e.g. foo*bar/baz (where * is acutally ANY_STRING)
+/// wc: the wildcard string itself, e.g. foo*bar/baz (where * is acutally ANY_CHAR)
 /// prefix: the string that should be prepended for completions that replace their token.
 //    This is usually the same thing as the original wildcard, but for fuzzy matching, we
 //    expand intermediate segments. effective_prefix is always either empty, or ends with a slash
@@ -800,7 +816,7 @@ void wildcard_expander_t::expand(const wcstring &base_dir, const wchar_t *wc,
     const size_t wc_segment_len = next_slash ? next_slash - wc : wc_len;
     const wcstring wc_segment = wcstring(wc, wc_segment_len);
     const bool segment_has_wildcards =
-        wildcard_has(wc_segment, true /* internal, i.e. look for ANY_STRING instead of * */);
+        wildcard_has(wc_segment, true /* internal, i.e. look for ANY_CHAR instead of ? */);
     const wchar_t *const wc_remainder = next_slash ? next_slash + 1 : NULL;
 
     if (wc_segment.empty()) {

src/wildcard.h

@@ -11,8 +11,10 @@
 
 // Enumeration of all wildcard types.
 enum {
+    /// Character representing any character except '/' (slash).
+    ANY_CHAR = WILDCARD_RESERVED_BASE,
     /// Character representing any character string not containing '/' (slash).
-    ANY_STRING = WILDCARD_RESERVED_BASE,
+    ANY_STRING,
     /// Character representing any character string.
     ANY_STRING_RECURSIVE,
     /// This is a special psuedo-char that is not used other than to mark the

tests/invocation/features-qmark1.invoke

@@ -0,0 +1 @@
+--features '' -c 'string match --quiet "??" ab ; echo "qmarkon: $status"'

tests/invocation/features-qmark1.out

@@ -0,0 +1 @@
+qmarkon: 0

tests/invocation/features-qmark2.invoke

@@ -0,0 +1 @@
+--features 'qmark-noglob' -C 'string match --quiet "??" ab ; echo "qmarkoff: $status"'

tests/invocation/features-qmark2.out

@@ -0,0 +1 @@
+qmarkoff: 1