Skip to content

Commit

Permalink
Address review comments:
Browse files Browse the repository at this point in the history
- CollatorInstantiation -> Collator (and try to keep Intl.Collator in its own scope)
- Collator holds onto its Intl.Collator now so it can be used across multiple compares
- Fix parse indexes
- Add explanatory comments
  • Loading branch information
ChrisLoer committed Apr 13, 2018
1 parent 2e107ba commit e3a9583
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 12 deletions.
26 changes: 16 additions & 10 deletions src/style-spec/expression/definitions/collator.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@ import type EvaluationContext from '../evaluation_context';
import type ParsingContext from '../parsing_context';
import type { Type } from '../types';

// Flow type declarations for Intl cribbed from
// https://github.com/facebook/flow/issues/1270

declare var Intl: {
Collator: Class<Collator>
Collator: Class<Intl$Collator>
}

declare class Collator {
declare class Intl$Collator {
constructor (
locales?: string | string[],
options?: CollatorOptions
Expand All @@ -36,9 +39,10 @@ type CollatorOptions = {
caseFirst?: 'upper' | 'lower' | 'false'
}

export class CollatorInstantiation {
export class Collator {
locale: string | null;
sensitivity: 'base' | 'accent' | 'case' | 'variant';
collator: Intl$Collator;

constructor(caseSensitive: boolean, diacriticSensitive: boolean, locale: string | null) {
if (caseSensitive)
Expand All @@ -47,16 +51,18 @@ export class CollatorInstantiation {
this.sensitivity = diacriticSensitive ? 'accent' : 'base';

this.locale = locale;
this.collator = (new Intl.Collator(this.locale ? this.locale : [],
{ sensitivity: this.sensitivity, usage: 'search' }): any);
}

compare(lhs: string, rhs: string): number {
return new Intl.Collator(this.locale ? this.locale : [],
{ sensitivity: this.sensitivity, usage: 'search' })
.compare(lhs, rhs);
return this.collator.compare(lhs, rhs);
}

resolvedLocale(): string {
return new Intl.Collator(this.locale ? this.locale : [])
// We create a Collator without "usage: search" because we don't want
// the search options encoded in our result (e.g. "en-u-co-search")
return (new Intl.Collator(this.locale ? this.locale : []): any)
.resolvedOptions().locale;
}

Expand Down Expand Up @@ -97,20 +103,20 @@ export class CollatorExpression implements Expression {
if (!caseSensitive) return null;

const diacriticSensitive = context.parse(
options['diacriticSensitive'] === undefined ? false : options['diacriticSensitive'], 2, BooleanType);
options['diacriticSensitive'] === undefined ? false : options['diacriticSensitive'], 1, BooleanType);
if (!diacriticSensitive) return null;

let locale = null;
if (options['locale']) {
locale = context.parse(options['locale'], 3, StringType);
locale = context.parse(options['locale'], 1, StringType);
if (!locale) return null;
}

return new CollatorExpression(caseSensitive, diacriticSensitive, locale);
}

evaluate(ctx: EvaluationContext) {
return new CollatorInstantiation(this.caseSensitive.evaluate(ctx), this.diacriticSensitive.evaluate(ctx), this.locale ? this.locale.evaluate(ctx) : null);
return new Collator(this.caseSensitive.evaluate(ctx), this.diacriticSensitive.evaluate(ctx), this.locale ? this.locale.evaluate(ctx) : null);
}

eachChild(fn: (Expression) => void) {
Expand Down
9 changes: 7 additions & 2 deletions src/style-spec/expression/definitions/literal.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import assert from 'assert';
import { isValue, typeOf } from '../values';
import Color from '../../util/color';
import { CollatorInstantiation } from '../definitions/collator';
import { Collator } from '../definitions/collator';

import type { Type } from '../types';
import type { Value } from '../values';
Expand Down Expand Up @@ -58,8 +58,13 @@ class Literal implements Expression {
if (this.type.kind === 'array' || this.type.kind === 'object') {
return ["literal", this.value];
} else if (this.value instanceof Color) {
// Constant-folding can generate Literal expressions that you
// couldn't actually generate with a "literal" expression,
// so we have to implement an equivalent serialization here
return ["rgba"].concat(this.value.toArray());
} else if (this.value instanceof CollatorInstantiation) {
} else if (this.value instanceof Collator) {
// Same as Color above: literal serialization delegated to
// Collator (not CollatorExpression)
return this.value.serialize();
} else {
assert(this.value === null ||
Expand Down

0 comments on commit e3a9583

Please sign in to comment.