Skip to content

Commit

Permalink
Return glob ordering to pre-3.1 state
Browse files Browse the repository at this point in the history
Glob ordering is used in a variety of places, including figuring out
conf.d and really needs to be stable.

Other ordering, like completions, is really just cosmetic and can
change if it makes for a nicer experience.

So we uncouple it by copying the wcsfilecmp from 3.0.2, which will
return the ordering to what it was in that release.

Fixes #6593
  • Loading branch information
faho committed Feb 14, 2020
1 parent 7c879ed commit f053cd2
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 3 deletions.
7 changes: 6 additions & 1 deletion src/expand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
#include "path.h"
#include "proc.h"
#include "reader.h"
#include "util.h"
#include "wcstringutil.h"
#include "wildcard.h"
#include "wutil.h" // IWYU pragma: keep
Expand Down Expand Up @@ -1031,7 +1032,11 @@ expand_result_t expander_t::stage_wildcards(wcstring path_to_expand, completion_
}
}

std::sort(expanded.begin(), expanded.end(), completion_t::is_naturally_less_than);
std::sort(expanded.begin(), expanded.end(),
[&](const completion_t &a, const completion_t &b) {
return wcsfilecmp_glob(a.completion.c_str(), b.completion.c_str()) < 0;
});

std::move(expanded.begin(), expanded.end(), std::back_inserter(*out));
} else {
// Can't fully justify this check. I think it's that SKIP_WILDCARDS is used when completing
Expand Down
49 changes: 49 additions & 0 deletions src/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,55 @@ int wcsfilecmp(const wchar_t *a, const wchar_t *b) {
return 1; // string b is a prefix of a and a is longer
}

/// wcsfilecmp, but frozen in time for glob usage.
int wcsfilecmp_glob(const wchar_t *a, const wchar_t *b) {
assert(a && b && "Null parameter");
const wchar_t *orig_a = a;
const wchar_t *orig_b = b;
int retval = 0; // assume the strings will be equal

while (*a && *b) {
if (iswdigit(*a) && iswdigit(*b)) {
retval = wcsfilecmp_leading_digits(&a, &b);
// If we know the strings aren't logically equal or we've reached the end of one or both
// strings we can stop iterating over the chars in each string.
if (retval || *a == 0 || *b == 0) break;
}

wint_t al = towlower(*a);
wint_t bl = towlower(*b);
if (al < bl) {
retval = -1;
break;
} else if (al > bl) {
retval = 1;
break;
} else {
a++;
b++;
}
}

if (retval != 0) return retval; // we already know the strings aren't logically equal

if (*a == 0) {
if (*b == 0) {
// The strings are logically equal. They may or may not be the same length depending on
// whether numbers were present but that doesn't matter. Disambiguate strings that
// differ by letter case or length. We don't bother optimizing the case where the file
// names are literally identical because that won't occur given how this function is
// used. And even if it were to occur (due to being reused in some other context) it
// would be so rare that it isn't worth optimizing for.
retval = wcscmp(orig_a, orig_b);
return retval < 0 ? -1 : retval == 0 ? 0 : 1;
}
return -1; // string a is a prefix of b and b is longer
}

assert(*b == 0);
return 1; // string b is a prefix of a and a is longer
}

/// Return microseconds since the epoch.
long long get_time() {
struct timeval time_struct;
Expand Down
3 changes: 3 additions & 0 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@
/// given above.
int wcsfilecmp(const wchar_t *a, const wchar_t *b);

/// wcsfilecmp, but frozen in time for glob usage.
int wcsfilecmp_glob(const wchar_t *a, const wchar_t *b);

/// Get the current time in microseconds since Jan 1, 1970.
long long get_time();

Expand Down
5 changes: 3 additions & 2 deletions tests/checks/wildcard.fish
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ set -l where ../test/temp/fish_wildcard_permissions_test/noaccess/yesaccess
mkdir -p $where
chmod 300 (dirname $where) # no read permissions
mkdir -p $where
touch $where/alpha.txt $where/beta.txt $where/delta.txt
# "__env.fish" here to confirm ordering - #6593.
touch $where/alpha.txt $where/beta.txt $where/delta.txt $where/__env.fish
echo $where/*
#CHECK: ../test/temp/fish_wildcard_permissions_test/noaccess/yesaccess/alpha.txt ../test/temp/fish_wildcard_permissions_test/noaccess/yesaccess/beta.txt ../test/temp/fish_wildcard_permissions_test/noaccess/yesaccess/delta.txt
#CHECK: ../test/temp/fish_wildcard_permissions_test/noaccess/yesaccess/__env.fish ../test/temp/fish_wildcard_permissions_test/noaccess/yesaccess/alpha.txt ../test/temp/fish_wildcard_permissions_test/noaccess/yesaccess/beta.txt ../test/temp/fish_wildcard_permissions_test/noaccess/yesaccess/delta.txt
chmod 700 (dirname $where) # so we can delete it
rm -rf ../test/temp/fish_wildcard_permissions_test

0 comments on commit f053cd2

Please sign in to comment.