Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make sort sensitivity opt-in and configurable #41

Merged
merged 7 commits into from
Jun 10, 2024
Merged

Make sort sensitivity opt-in and configurable #41

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c9e0514
Make sort sensitivity opt-in and configurable
Mar 8, 2024
aa85c6f
Fix type of --sort-sensitivity parameter in Readme
Mar 8, 2024
0e68a1b
Merge branch 'vendure-ecommerce:master' into feature/allow-case-sensi…
bjarne-callewaert Mar 8, 2024
6b72ec6
Update unit tests for SortByKeyPostProcessor for better readability a…
Jun 4, 2024
53755f4
Add README section dedicated to sorting of extracted keys
Jun 4, 2024
572f83b
Merge branch 'master' into feature/allow-case-sensitive-translation-k…
Jun 4, 2024
3f38fd2
Fix type in Readme
Jun 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ Output
--format, -f Format [string] [choices: "json", "namespaced-json", "pot"] [default: "json"]
--format-indentation, --fi Format indentation (JSON/Namedspaced JSON) [string] [default: "\t"]
--sort, -s Sort strings in alphabetical order [boolean]
--sort-sensitivity, -ss Sensitivity when sorting strings [string]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to add a dedicated section to the readme about sorting now. If I understand correctly, the -ss flag only has an effect in combination with the -s flag right? That should be noted.

Also this section can explain how strings will be sorted without the -ss flag, as well as what the different options are and what effect they have on the sort order.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Limitations for the -ss flag have been added, as well as a separate section in the README

--clean, -c Remove obsolete strings after merge [boolean]
--replace, -r Replace the contents of output file if it exists (Merges by default) [boolean]
--strip-prefix, -sp Strip prefix from key [string]
Expand Down
11 changes: 9 additions & 2 deletions src/cli/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,13 @@ const cli = await y
describe: 'Sort strings in alphabetical order',
type: 'boolean'
})
.option('sort-sensitivity', {
alias: 'ss',
describe: 'Sort sensitivitiy of strings',
type: 'string',
choices: ['base', 'accent', 'case', 'variant'],
default: undefined
})
.option('clean', {
alias: 'c',
describe: 'Remove obsolete strings after merge',
Expand Down Expand Up @@ -118,7 +125,7 @@ const cli = await y
describe: 'Strip a prefix from the extracted key',
type: 'string'
})
.group(['format', 'format-indentation', 'sort', 'clean', 'replace', 'strip-prefix'], 'Output')
.group(['format', 'format-indentation', 'sort', 'sort-sensitivity', 'clean', 'replace', 'strip-prefix'], 'Output')
.group(['key-as-default-value', 'null-as-default-value', 'string-as-default-value'], 'Extracted key value (defaults to empty string)')
.conflicts('key-as-default-value', 'null-as-default-value')
.example('$0 -i ./src-a/ -i ./src-b/ -o strings.json', 'Extract (ts, html) from multiple paths')
Expand Down Expand Up @@ -167,7 +174,7 @@ if (cli.stripPrefix) {
}

if (cli.sort) {
postProcessors.push(new SortByKeyPostProcessor());
postProcessors.push(new SortByKeyPostProcessor(cli.sortSensitivity));
}
extractTask.setPostProcessors(postProcessors);

Expand Down
16 changes: 15 additions & 1 deletion src/post-processors/sort-by-key.post-processor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,22 @@ import { PostProcessorInterface } from './post-processor.interface.js';
export class SortByKeyPostProcessor implements PostProcessorInterface {
public name: string = 'SortByKey';

public sortSensitivity: 'base' | 'accent' | 'case' | 'variant' | undefined = undefined;

constructor(sortSensitivity: string | undefined) {
if (isOfTypeSortSensitivity(sortSensitivity)) {
this.sortSensitivity = sortSensitivity;
} else {
throw new Error(`Unknown sortSensitivity: ${sortSensitivity}`);
}
}

public process(draft: TranslationCollection, extracted: TranslationCollection, existing: TranslationCollection): TranslationCollection {
const compareFn = new Intl.Collator('en', {sensitivity: 'base'}).compare;
const compareFn = this.sortSensitivity ? new Intl.Collator('en', { sensitivity: this.sortSensitivity }).compare : undefined;
return draft.sort(compareFn);
}
}

function isOfTypeSortSensitivity(keyInput: string | undefined): keyInput is 'base' | 'accent' | 'case' | 'variant' | undefined {
return ['base', 'accent', 'case', 'variant'].includes(keyInput) || keyInput === undefined;
}
122 changes: 120 additions & 2 deletions tests/post-processors/sort-by-key.post-processor.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import { PostProcessorInterface } from '../../src/post-processors/post-processor
import { SortByKeyPostProcessor } from '../../src/post-processors/sort-by-key.post-processor.js';
import { TranslationCollection } from '../../src/utils/translation.collection.js';

describe('SortByKeyPostProcessor', () => {
describe('SortByKeyPostProcessor - base sensitivity should sort lowercase and uppercase equally', () => {
let processor: PostProcessorInterface;

beforeEach(() => {
processor = new SortByKeyPostProcessor();
processor = new SortByKeyPostProcessor('base');
});

it('should sort keys alphanumerically', () => {
Expand All @@ -27,6 +27,15 @@ describe('SortByKeyPostProcessor', () => {
b: {value: 'another value', sourceFiles: []},
z: {value: 'last value', sourceFiles: []}
});

expect(Object.keys(processor.process(collection, extracted, existing).values)).to.deep.equal(
Object.keys({
'9': {value: 'a numeric key', sourceFiles: [] },
a: {value: 'a value', sourceFiles: [] },
b: {value: 'another value', sourceFiles: [] },
z: {value: 'last value', sourceFiles: [] }
})
);
});

it('should perform case insensitive sorting', () => {
Expand All @@ -47,6 +56,96 @@ describe('SortByKeyPostProcessor', () => {
g: { value: 'letter g', sourceFiles: [] }
});

expect(processor.process(collection, new TranslationCollection(), new TranslationCollection()).values).to.deep.equal({
c: {value: 'letter c', sourceFiles: [] },
j: {value: 'letter j', sourceFiles: [] },
b: {value: 'letter b', sourceFiles: [] },
a: {value: 'letter a', sourceFiles: [] },
h: {value: 'letter h', sourceFiles: [] },
B: {value: 'letter B', sourceFiles: [] },
H: {value: 'letter H', sourceFiles: [] },
i: {value: 'letter i', sourceFiles: [] },
C: {value: 'letter C', sourceFiles: [] },
e: {value: 'letter e', sourceFiles: [] },
f: {value: 'letter f', sourceFiles: [] },
d: {value: 'letter d', sourceFiles: [] },
A: {value: 'letter A', sourceFiles: [] },
g: {value: 'letter g', sourceFiles: [] }
});

expect(Object.keys(processor.process(collection, new TranslationCollection(), new TranslationCollection()).values)).to.deep.equal(
Object.keys({
a: {value: 'letter a', sourceFiles: [] },
A: {value: 'letter A', sourceFiles: [] },
b: {value: 'letter b', sourceFiles: [] },
B: {value: 'letter B', sourceFiles: [] },
c: {value: 'letter c', sourceFiles: [] },
C: {value: 'letter C', sourceFiles: [] },
d: {value: 'letter d', sourceFiles: [] },
e: {value: 'letter e', sourceFiles: [] },
f: {value: 'letter f', sourceFiles: [] },
g: {value: 'letter g', sourceFiles: [] },
h: {value: 'letter h', sourceFiles: [] },
H: {value: 'letter H', sourceFiles: [] },
i: {value: 'letter i', sourceFiles: [] },
j: {value: 'letter j', sourceFiles: [] }
})
);
});
});

describe('SortByKeyPostProcessor - undefined sort sensitivity should sort lowercase and uppercase separatly', () => {
let processor: PostProcessorInterface;

beforeEach(() => {
processor = new SortByKeyPostProcessor(undefined);
});

it('should sort keys alphanumerically', () => {
const collection = new TranslationCollection({
z: {value: 'last value', sourceFiles: [] },
a: {value: 'a value', sourceFiles: [] },
'9': {value: 'a numeric key', sourceFiles: [] },
b: {value: 'another value', sourceFiles: [] }
});
const extracted = new TranslationCollection();
const existing = new TranslationCollection();

expect(processor.process(collection, extracted, existing).values).to.deep.equal({
'9': {value: 'a numeric key', sourceFiles: [] },
a: {value: 'a value', sourceFiles: [] },
b: {value: 'another value', sourceFiles: [] },
z: {value: 'last value', sourceFiles: [] }
});

expect(Object.keys(processor.process(collection, extracted, existing).values)).to.deep.equal(
Object.keys({
'9': {value: 'a numeric key', sourceFiles: [] },
a: {value: 'a value', sourceFiles: [] },
b: {value: 'another value', sourceFiles: [] },
z: {value: 'last value', sourceFiles: [] }
})
);
});

it('should perform case sensitive sorting', () => {
const collection = new TranslationCollection({
c: {value: 'letter c', sourceFiles: [] },
j: {value: 'letter j', sourceFiles: [] },
b: {value: 'letter b', sourceFiles: [] },
a: {value: 'letter a', sourceFiles: [] },
h: {value: 'letter h', sourceFiles: [] },
B: {value: 'letter B', sourceFiles: [] },
H: {value: 'letter H', sourceFiles: [] },
i: {value: 'letter i', sourceFiles: [] },
C: {value: 'letter C', sourceFiles: [] },
e: {value: 'letter e', sourceFiles: [] },
f: {value: 'letter f', sourceFiles: [] },
d: {value: 'letter d', sourceFiles: [] },
A: {value: 'letter A', sourceFiles: [] },
g: {value: 'letter g', sourceFiles: [] }
});

expect(processor.process(collection, new TranslationCollection(), new TranslationCollection()).values).to.deep.equal({
A: { value: 'letter A', sourceFiles: [] },
a: { value: 'letter a', sourceFiles: [] },
Expand All @@ -63,5 +162,24 @@ describe('SortByKeyPostProcessor', () => {
i: { value: 'letter i', sourceFiles: [] },
j: { value: 'letter j', sourceFiles: [] }
});

expect(Object.keys(processor.process(collection, new TranslationCollection(), new TranslationCollection()).values)).to.deep.equal(
Object.keys({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what this assertion is testing beyond what the test above it is testing? Can you clarify this?

Copy link
Author

@bjarne-callewaert bjarne-callewaert Jun 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to my findings, the to.deep.equal does not assert the order in Objects.
These tests were added quite recently by this PR: #35 but these these have not been designed to verify the order of the extracted keys.

I modified the assertion a bit and added some comments to make this more clear

Additionally, some tests were added to have at least one assertion per sort-sensitivity option

A: {value: 'letter A', sourceFiles: [] },
B: {value: 'letter B', sourceFiles: [] },
C: {value: 'letter C', sourceFiles: [] },
H: {value: 'letter H', sourceFiles: [] },
a: {value: 'letter a', sourceFiles: [] },
b: {value: 'letter b', sourceFiles: [] },
c: {value: 'letter c', sourceFiles: [] },
d: {value: 'letter d', sourceFiles: [] },
e: {value: 'letter e', sourceFiles: [] },
f: {value: 'letter f', sourceFiles: [] },
g: {value: 'letter g', sourceFiles: [] },
h: {value: 'letter h', sourceFiles: [] },
i: {value: 'letter i', sourceFiles: [] },
j: {value: 'letter j', sourceFiles: [] }
})
);
});
});
Loading