Skip to content

Commit a17322f

Browse files
committed
Use the calculated cell length instead of string length for the separator length
1 parent d0c8dd2 commit a17322f

9 files changed

Lines changed: 73 additions & 31 deletions

File tree

src/viewModelFactories/rowViewModelFactory.ts

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,32 +14,34 @@ export class RowViewModelFactory {
1414
public buildRow(row: number, table: Table): RowViewModel {
1515
if (table == null) throw new Error("Paramter can't be null");
1616

17-
let resultRow = new Array(table.columnCount);
17+
let resultRow: string[] = new Array(table.columnCount);
18+
let displayWidths: number[] = new Array(table.columnCount);
1819

1920
for(let col = 0; col < table.columnCount; col++) {
20-
resultRow[col] =
21-
this._contentPadCalculator.getLeftPadding(table, row, col) +
22-
table.rows[row].cells[col].getValue() +
23-
this._contentPadCalculator.getRightPadding(table, row, col);
21+
const leftPadding = this._contentPadCalculator.getLeftPadding(table, row, col);
22+
const rightPadding = this._contentPadCalculator.getRightPadding(table, row, col);
23+
24+
resultRow[col] = leftPadding + table.rows[row].cells[col].getValue() + rightPadding;
25+
displayWidths[col] = leftPadding.length + table.rows[row].cells[col].getLength() + rightPadding.length;
2426
}
2527

26-
return new RowViewModel(resultRow, table.rows[row].EOL);
28+
return new RowViewModel(resultRow, displayWidths, table.rows[row].EOL);
2729
}
2830

2931
public buildSeparator(rows: RowViewModel[], table: Table): RowViewModel {
30-
const columnCount = rows[0].columnCount;
31-
let lengths = Array(columnCount).fill(0);
32+
const columnCount: number = rows[0].columnCount;
33+
let lengths: number[] = Array(columnCount).fill(0);
3234

3335
for (const row of rows) {
34-
for (let i = 0; i < columnCount; i++) {
35-
lengths[i] = Math.max(lengths[i], (row.getValueAt(i).length));
36+
for (let col = 0; col < columnCount; col++) {
37+
lengths[col] = Math.max(lengths[col], row.getDisplayWidthAt(col));
3638
}
3739
}
3840

3941
const values: string[] = lengths
4042
.map(l => this.separatorChar.repeat(l))
4143
.map((val, col) => this._alignmentMarkerStrategy.markerFor(table.alignments[col]).mark(val));
4244

43-
return new RowViewModel(values, table.separatorEOL);
45+
return new RowViewModel(values, lengths, table.separatorEOL);
4446
}
4547
}

src/viewModels/rowViewModel.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,13 @@ export class RowViewModel {
22

33
constructor (
44
private readonly _values: string[],
5+
private readonly _displayWidhts: number[],
56
private readonly _eol: string
6-
) { }
7+
) {
8+
if (_values == null) throw new Error("Parameter can't be null");
9+
if (_displayWidhts == null) throw new Error("Parameter can't be null");
10+
if (_values.length !== _displayWidhts.length) throw new Error("Values and display widths must have the same length.");
11+
}
712

813
public get columnCount(): number { return this._values.length; }
914
public get EOL(): string { return this._eol; }
@@ -15,4 +20,12 @@ export class RowViewModel {
1520

1621
return this._values[index];
1722
}
23+
24+
public getDisplayWidthAt(index: number): number {
25+
const maxIndex = this._displayWidhts.length - 1;
26+
if (index < 0 || index > maxIndex)
27+
throw new Error(`Argument out of range; should be between 0 and ${maxIndex}, but was ${index}.`);
28+
29+
return this._displayWidhts[index];
30+
}
1831
}

test/systemTests/resources/cjkHaveLength2-expected.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,9 @@ ushort | 0 to 65,535 | Unsigned 16
88
int 𠁻 | -2,147,483,648 to 2,147,483,647 | Signed 32-bit integer
99
uint | 0 to 4,294,967,295 | Unsigned 32-bit integer
1010
long | -9,223,372,036,854,775,808 to 9,223,372,036,854,775,807 | Signed 64-bit integer
11-
ulong 𠁻 | 0 to 18,446,744,073,709,551,615 | Unsigned 64-bit integer
11+
ulong 𠁻 | 0 to 18,446,744,073,709,551,615 | Unsigned 64-bit integer
12+
13+
| **模組** | **Second** |
14+
|----------|------------|
15+
| 模具設計 | V |
16+
| 零件加工 | X |

test/systemTests/resources/cjkHaveLength2-input.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,9 @@ ushort| 0 to 65,535| Unsigned 16-bit integer
88
int 𠁻| -2,147,483,648 to 2,147,483,647| Signed 32-bit integer
99
uint| 0 to 4,294,967,295| Unsigned 32-bit integer
1010
long| -9,223,372,036,854,775,808 to 9,223,372,036,854,775,807| Signed 64-bit integer
11-
ulong 𠁻| 0 to 18,446,744,073,709,551,615| Unsigned 64-bit integer
11+
ulong 𠁻| 0 to 18,446,744,073,709,551,615| Unsigned 64-bit integer
12+
13+
| **模組** | **Second** |
14+
|--|--|
15+
| 模具設計 | V |
16+
| 零件加工 | X |

test/unitTests/models/cell.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,16 @@ suite("Cell tests", () => {
3434
test("getLength() for CJK characters returns 2 for each char", () => {
3535
assert.strictEqual(new Cell("test - 𠁻 𣄿 𣄿 content").getLength(), 23);
3636
});
37+
38+
test("getLength() for specific CJK characters 1", () => {
39+
assert.strictEqual(new Cell("模組").getLength(), 4);
40+
});
41+
42+
test("getLength() for specific CJK characters 2", () => {
43+
assert.strictEqual(new Cell("模具設計").getLength(), 8);
44+
});
45+
46+
test("getLength() for specific CJK characters 3", () => {
47+
assert.strictEqual(new Cell("零件加工").getLength(), 8);
48+
});
3749
});

test/unitTests/viewModelFactories/rowViewModelFactory.test.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ suite("RowViewModelFactory.buildRow() tests", () => {
2121
const sut = createFactory(_contentPadCalculator.object);
2222
const row = 1;
2323
const table = threeColumnTable();
24+
_contentPadCalculator.setup(_ => _.getLeftPadding(It.isAny(), It.isAny(), It.isAny())).returns(() => "");
25+
_contentPadCalculator.setup(_ => _.getRightPadding(It.isAny(), It.isAny(), It.isAny())).returns(() => "");
2426

2527
sut.buildRow(row, table);
2628

@@ -37,22 +39,24 @@ suite("RowViewModelFactory.buildRow() tests", () => {
3739
const sut = createFactory(_contentPadCalculator.object);
3840
const row = 1;
3941
const table = threeColumnTable();
40-
_contentPadCalculator.setup(_ => _.getLeftPadding(It.isAny(), It.isAny(), It.isAny())).returns(() => "test");
42+
_contentPadCalculator.setup(_ => _.getLeftPadding(It.isAny(), It.isAny(), It.isAny())).returns(() => "test1");
43+
_contentPadCalculator.setup(_ => _.getRightPadding(It.isAny(), It.isAny(), It.isAny())).returns(() => "test2");
4144

4245
const rowViewModel = sut.buildRow(row, table);
4346

44-
assert.strictEqual(rowViewModel.getValueAt(1).startsWith("test"), true);
47+
assert.strictEqual(rowViewModel.getValueAt(1).startsWith("test1"), true);
4548
});
4649

4750
test("Value returned from padCalculator.getRightPadding is used to end the row value", () => {
4851
const sut = createFactory(_contentPadCalculator.object);
4952
const row = 1;
5053
const table = threeColumnTable();
51-
_contentPadCalculator.setup(_ => _.getRightPadding(It.isAny(), row, It.isAny())).returns(() => "test");
54+
_contentPadCalculator.setup(_ => _.getLeftPadding(It.isAny(), row, It.isAny())).returns(() => "test1");
55+
_contentPadCalculator.setup(_ => _.getRightPadding(It.isAny(), row, It.isAny())).returns(() => "test2");
5256

5357
const rowViewModel = sut.buildRow(row, table);
5458

55-
assert.strictEqual(rowViewModel.getValueAt(1).endsWith("test"), true);
59+
assert.strictEqual(rowViewModel.getValueAt(1).endsWith("test2"), true);
5660
});
5761

5862
test("Empty middle column uses only left and right pad to create the value", () => {
@@ -77,12 +81,12 @@ suite("RowViewModelFactory.buildSeparator() tests", () => {
7781
_contentPadCalculator = Mock.ofType<PadCalculator>();
7882
});
7983

80-
test("Fills separator with dashes using the max length of the columns", () => {
84+
test("Fills separator with dashes using the max length of the displayWidths", () => {
8185
const sut = createFactory(_contentPadCalculator.object);
8286
const table = threeColumnTable();
8387
const rows: RowViewModel[] = [
84-
new RowViewModel(["abc", "defghi"], "\n"),
85-
new RowViewModel(["abcd", "efgh"], "\n")
88+
new RowViewModel(["abc", "defghi"], [3, 6], "\n"),
89+
new RowViewModel(["abcd", "efgh"], [4, 4], "\n")
8690
];
8791

8892
const separator = sut.buildSeparator(rows, table);
@@ -100,8 +104,8 @@ suite("RowViewModelFactory.buildSeparator() tests", () => {
100104
const sut = createFactory(_contentPadCalculator.object, alignmentStrategy.object);
101105
const table = threeColumnTable();
102106
const rows: RowViewModel[] = [
103-
new RowViewModel(["abc", "defghi", "xyx"], "\n"),
104-
new RowViewModel(["abcd", "efgh", "xyz"], "\n")
107+
new RowViewModel(["abc", "defghi", "xyx"], [3, 6, 3], "\n"),
108+
new RowViewModel(["abcd", "efgh", "xyz"], [4, 4, 3], "\n")
105109
];
106110

107111
sut.buildSeparator(rows, table);

test/unitTests/viewModelFactories/tableViewModelFactory.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ suite("TableViewModelFactory tests", () => {
2323
["v1", "v2"],
2424
["v3", "v4"],
2525
]);
26-
const expectedSeparator = new RowViewModel([], "");
27-
const expectedRow = new RowViewModel([], "");
26+
const expectedSeparator = new RowViewModel([], [], "");
27+
const expectedRow = new RowViewModel([], [], "");
2828

2929
_rowVmb
3030
.setup(m => m.buildSeparator(It.isAny(), It.isAny()))
@@ -47,8 +47,8 @@ suite("TableViewModelFactory tests", () => {
4747
["v1", "v2"],
4848
["v3", "v4"],
4949
]);
50-
const expectedSeparator = new RowViewModel([], "");
51-
const expectedRow = new RowViewModel([], "");
50+
const expectedSeparator = new RowViewModel([], [], "");
51+
const expectedRow = new RowViewModel([], [], "");
5252
_rowVmb.setup(m => m.buildSeparator(It.isAny(), It.isAny())).returns(() => expectedSeparator)
5353
_rowVmb.setup(m => m.buildRow(It.isAny(), It.isAny())).returns(() => expectedRow);
5454

test/unitTests/writers/tableStringWriter.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,8 @@ suite("TableStringWriter tests", () => {
195195
});
196196

197197
function makeRowViewModel(values: string[]) {
198-
return new RowViewModel(values, eol);
198+
const displayWidths: number[] = values.map(v => v.length);
199+
return new RowViewModel(values, displayWidths, eol);
199200
}
200201

201202
function createSut() : TableStringWriter {

test/unitTests/writers/valuePaddingProvider.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ suite("ValuePaddingProvider tests", () => {
1616
test("getRightPadding() returns the pad for middle columns", () => {
1717
const sut = new ValuePaddingProvider(3);
1818
const table = new TableViewModel();
19-
table.header = new RowViewModel(["", "", "", ""], "\n");
19+
table.header = new RowViewModel(["", "", "", ""], [0, 0, 0, 0], "\n");
2020

2121
const pad = sut.getRightPadding(table, 2);
2222

@@ -27,7 +27,7 @@ suite("ValuePaddingProvider tests", () => {
2727
const sut = new ValuePaddingProvider(3);
2828
const table = new TableViewModel();
2929
table.hasRightBorder = true;
30-
table.header = new RowViewModel(["", "", "", ""], "\n");
30+
table.header = new RowViewModel(["", "", "", ""], [0, 0, 0, 0], "\n");
3131

3232
const pad = sut.getRightPadding(table, 3);
3333

@@ -38,7 +38,7 @@ suite("ValuePaddingProvider tests", () => {
3838
const sut = new ValuePaddingProvider(3);
3939
const table = new TableViewModel();
4040
table.hasRightBorder = false;
41-
table.header = new RowViewModel(["", "", "", ""], "\n");
41+
table.header = new RowViewModel(["", "", "", ""], [0, 0, 0, 0], "\n");
4242

4343
const pad = sut.getRightPadding(table, 3);
4444

0 commit comments

Comments
 (0)