(core) Use unicode-aware comparisons for user-visible strings.

Summary:
- Switch code that compares user strings to use localeCompare() based on Intl.Collator.
- Use en-US locale for now. (Ideally should be a document property.)
- Note that with this change, sorting is also becoming case-insensitive (which
  seems an improvement)

- Updated a sorted test fixture
- Updated a browser test with lots of unicode to expect different order.
- Added a bit of unicode to test ordering in Reference autocomplete dropdown.

Test Plan: Fixed / updated tests

Reviewers: paulfitz

Reviewed By: paulfitz

Differential Revision: https://phab.getgrist.com/D2758
This commit is contained in:
Dmitry S 2021-03-12 21:25:44 -05:00
parent 85a2492123
commit 6e844a2e76
5 changed files with 35 additions and 19 deletions

View File

@ -7,7 +7,7 @@
* "lush" would only match the "L" in "Lavender". * "lush" would only match the "L" in "Lavender".
*/ */
import {nativeCompare, sortedIndex} from 'app/common/gutil'; import {localeCompare, nativeCompare, sortedIndex} from 'app/common/gutil';
import {DomContents} from 'grainjs'; import {DomContents} from 'grainjs';
export interface ACItem { export interface ACItem {
@ -80,7 +80,7 @@ export class ACIndexImpl<Item extends ACItem> implements ACIndex<Item> {
} }
} }
allWords.sort((a, b) => nativeCompare(a.word, b.word)); allWords.sort((a, b) => localeCompare(a.word, b.word));
this._words = allWords; this._words = allWords;
} }

View File

@ -13,7 +13,7 @@ import {UserError} from 'app/client/models/errors';
import {TableData} from 'app/client/models/TableData'; import {TableData} from 'app/client/models/TableData';
import {DocAction} from 'app/common/DocActions'; import {DocAction} from 'app/common/DocActions';
import {isBulkUpdateRecord, isUpdateRecord} from 'app/common/DocActions'; import {isBulkUpdateRecord, isUpdateRecord} from 'app/common/DocActions';
import {getSetMapValue, nativeCompare} from 'app/common/gutil'; import {getSetMapValue, localeCompare, nativeCompare} from 'app/common/gutil';
import {BaseFormatter} from 'app/common/ValueFormatter'; import {BaseFormatter} from 'app/common/ValueFormatter';
export interface ICellItem { export interface ICellItem {
@ -76,7 +76,7 @@ export class ColumnACIndexes {
} }
function itemCompare(a: ICellItem, b: ICellItem) { function itemCompare(a: ICellItem, b: ICellItem) {
return nativeCompare(a.cleanText, b.cleanText) || return localeCompare(a.cleanText, b.cleanText) ||
nativeCompare(a.text, b.text) || localeCompare(a.text, b.text) ||
nativeCompare(a.rowId, b.rowId); nativeCompare(a.rowId, b.rowId);
} }

View File

@ -15,7 +15,7 @@ import {colors, vars} from 'app/client/ui2018/cssVars';
import {icon} from 'app/client/ui2018/icons'; import {icon} from 'app/client/ui2018/icons';
import {menuCssClass, menuDivider, menuIcon} from 'app/client/ui2018/menus'; import {menuCssClass, menuDivider, menuIcon} from 'app/client/ui2018/menus';
import {CellValue} from 'app/common/DocActions'; import {CellValue} from 'app/common/DocActions';
import {nativeCompare} from 'app/common/gutil'; import {localeCompare} from 'app/common/gutil';
import {Computed, dom, input, makeTestId, Observable, styled} from 'grainjs'; import {Computed, dom, input, makeTestId, Observable, styled} from 'grainjs';
import escapeRegExp = require('lodash/escapeRegExp'); import escapeRegExp = require('lodash/escapeRegExp');
import identity = require('lodash/identity'); import identity = require('lodash/identity');
@ -63,7 +63,7 @@ export function columnFilterMenu({ columnFilter, valueCounts, doSave, onClose }:
const filteredValues = Computed.create(null, openSearch, searchValueObs, (_use, isOpen, searchValue) => { const filteredValues = Computed.create(null, openSearch, searchValueObs, (_use, isOpen, searchValue) => {
const searchRegex = new RegExp(escapeRegExp(searchValue), 'i'); const searchRegex = new RegExp(escapeRegExp(searchValue), 'i');
return valueCountArr.filter(([key]) => !isOpen || searchRegex.test(key as string)) return valueCountArr.filter(([key]) => !isOpen || searchRegex.test(key as string))
.sort((a, b) => nativeCompare(a[1].label, b[1].label)); .sort((a, b) => localeCompare(a[1].label, b[1].label));
}); });
let searchInput: HTMLInputElement; let searchInput: HTMLInputElement;

View File

@ -7,7 +7,7 @@
* currently implemented. * currently implemented.
*/ */
import {ColumnGetters} from 'app/common/ColumnGetters'; import {ColumnGetters} from 'app/common/ColumnGetters';
import {nativeCompare} from 'app/common/gutil'; import {localeCompare, nativeCompare} from 'app/common/gutil';
/** /**
* Compare two cell values, paying attention to types and values. Note that native JS comparison * Compare two cell values, paying attention to types and values. Note that native JS comparison
@ -19,16 +19,27 @@ import {nativeCompare} from 'app/common/gutil';
* because e.g. a numerical column may contain text (alttext) or null values. * because e.g. a numerical column may contain text (alttext) or null values.
*/ */
export function typedCompare(val1: any, val2: any): number { export function typedCompare(val1: any, val2: any): number {
// TODO: We should use Intl.Collator for string comparisons to handle accented strings. let result: number, type1: string, array1: boolean;
let type1, array1; // tslint:disable-next-line:no-conditional-assignment
return nativeCompare(type1 = typeof val1, typeof val2) || if ((result = nativeCompare(type1 = typeof val1, typeof val2)) !== 0) {
return result;
}
// We need to worry about Array comparisons because formulas returing Any may return null or // We need to worry about Array comparisons because formulas returing Any may return null or
// object values represented as arrays (e.g. ['D', ...] for dates). Comparing those without // object values represented as arrays (e.g. ['D', ...] for dates). Comparing those without
// distinguishing types would break the sort. Also, arrays need a special comparator. // distinguishing types would break the sort. Also, arrays need a special comparator.
(type1 === 'object' && if (type1 === 'object') {
(nativeCompare(array1 = val1 instanceof Array, val2 instanceof Array) || // tslint:disable-next-line:no-conditional-assignment
(array1 && _arrayCompare(val1, val2)))) || if ((result = nativeCompare(array1 = val1 instanceof Array, val2 instanceof Array)) !== 0) {
nativeCompare(val1, val2); return result;
}
if (array1) {
return _arrayCompare(val1, val2);
}
}
if (type1 === 'string') {
return localeCompare(val1, val2);
}
return nativeCompare(val1, val2);
} }
function _arrayCompare(val1: any[], val2: any[]): number { function _arrayCompare(val1: any[], val2: any[]): number {
@ -36,7 +47,7 @@ function _arrayCompare(val1: any[], val2: any[]): number {
if (i >= val2.length) { if (i >= val2.length) {
return 1; return 1;
} }
const value = nativeCompare(val1[i], val2[i]); const value = typedCompare(val1[i], val2[i]);
if (value) { if (value) {
return value; return value;
} }

View File

@ -444,6 +444,11 @@ export function nativeCompare<T>(a: T, b: T): number {
return (a < b ? -1 : (a > b ? 1 : 0)); return (a < b ? -1 : (a > b ? 1 : 0));
} }
// TODO: In the future, locale should be a value associated with the document or the user.
export const defaultLocale = 'en-US';
export const defaultCollator = new Intl.Collator(defaultLocale);
export const localeCompare = defaultCollator.compare;
/** /**
* A copy of python`s `setdefault` function. * A copy of python`s `setdefault` function.
* Sets key in mapInst to value, if key is not already set. * Sets key in mapInst to value, if key is not already set.