Skip to content

Commit

Permalink
Fix table rendering
Browse files Browse the repository at this point in the history
Calculcate column widths so that the table will not overflow the
terminal anymore.

Switch back to 'table' library which now has newline support and not the
rendering bug that 'cli-table' had.
  • Loading branch information
rix0rrr committed Jan 14, 2019
1 parent 07264c4 commit f0975c1
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 47 deletions.
111 changes: 111 additions & 0 deletions packages/@aws-cdk/cloudformation-diff/lib/format-table.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import colors = require('colors/safe');
import stringWidth = require('string-width');
import table = require('table');

/**
* Render a two-dimensional array to a visually attractive table
*
* First row is considered the table header.
*/
export function formatTable(cells: string[][], columns: number | undefined): string {
return table.table(cells, {
border: TABLE_BORDER_CHARACTERS,
columns: buildColumnConfig(calculcateColumnWidths(cells, columns)),
drawHorizontalLine: (line) => {
// Numbering like this: [line 0] [header = row[0]] [line 1] [row 1] [line 2] [content 2] [line 3]
return (line < 2 || line === cells.length) || lineBetween(cells[line - 1], cells[line]);
}
});
}

/**
* Whether we should draw a line between two rows
*
* Draw horizontal line if 2nd column values are different.
*/
function lineBetween(rowA: string[], rowB: string[]) {
return rowA[1] !== rowB[1];
}

function buildColumnConfig(widths: Array<number | undefined>): { [index: number]: table.ColumnConfig } {
const ret: { [index: number]: table.ColumnConfig } = {};
widths.forEach((width, i) => {
ret[i] = { width, useWordWrap: true } as any; // 'useWordWrap' is not in @types/table
if (width === undefined) {
delete ret[i].width;
}
});

return ret;
}

/**
* Calculate column widths given a terminal width
*
* We do this by calculating a fair share for every column. Extra width smaller
* than the fair share is evenly distributed over all columns that exceed their
* fair share.
*/
function calculcateColumnWidths(rows: string[][], terminalWidth: number | undefined): Array<number | undefined> {
// use 'string-width' to not count ANSI chars as actual character width
const columns = rows[0].map((_, i) => Math.max(...rows.map(row => stringWidth(row[i]))));

// If we have no terminal width, do nothing
if (terminalWidth === undefined) { return columns.map(_ => undefined); }

const contentWidth = terminalWidth - 2 - columns.length * 3;

// If we don't exceed the terminal width, do nothing
if (sum(columns) <= contentWidth) { return columns.map(_ => undefined); }

const fairShare = Math.min(contentWidth / columns.length);
const smallColumns = columns.filter(w => w < fairShare);

let distributableWidth = contentWidth - sum(smallColumns);
const fairDistributable = Math.floor(distributableWidth / (columns.length - smallColumns.length));

const ret = new Array<number>();
for (const requestedWidth of columns) {
if (requestedWidth < fairShare) {
// Small column gets what they want
ret.push(requestedWidth);
} else {
// Last column gets all remaining, otherwise get fair redist share
const width = distributableWidth < 2 * fairDistributable ? distributableWidth : fairDistributable;
ret.push(width);
distributableWidth -= width;
}
}

return ret;
}

function sum(xs: number[]): number {
let total = 0;
for (const x of xs) {
total += x;
}
return total;
}

// What color the table is going to be
const tableColor = colors.gray;

// Unicode table characters with a color
const TABLE_BORDER_CHARACTERS = {
topBody: tableColor('─'),
topJoin: tableColor('┬'),
topLeft: tableColor('┌'),
topRight: tableColor('┐'),
bottomBody: tableColor('─'),
bottomJoin: tableColor('┴'),
bottomLeft: tableColor('└'),
bottomRight: tableColor('┘'),
bodyLeft: tableColor('│'),
bodyRight: tableColor('│'),
bodyJoin: tableColor('│'),
joinBody: tableColor('─'),
joinLeft: tableColor('├'),
joinRight: tableColor('┤'),
joinJoin: tableColor('┼')
};
49 changes: 4 additions & 45 deletions packages/@aws-cdk/cloudformation-diff/lib/format.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import cxapi = require('@aws-cdk/cx-api');
import Table = require('cli-table');
import colors = require('colors/safe');
import { format } from 'util';
import { Difference, isPropertyDifference, ResourceDifference, ResourceImpact } from './diff-template';
import { DifferenceCollection, TemplateDiff } from './diff/types';
import { deepEqual } from './diff/util';
import { formatTable } from './format-table';
import { IamChanges } from './iam/iam-changes';
import { SecurityGroupChanges } from './network/security-group-changes';

Expand Down Expand Up @@ -352,20 +352,20 @@ class Formatter {

if (changes.statements.hasChanges) {
this.printSectionHeader('IAM Statement Changes');
this.print(renderTable(this.deepSubstituteBracedLogicalIds(changes.summarizeStatements())));
this.print(formatTable(this.deepSubstituteBracedLogicalIds(changes.summarizeStatements()), this.stream.columns));
}

if (changes.managedPolicies.hasChanges) {
this.printSectionHeader('IAM Policy Changes');
this.print(renderTable(this.deepSubstituteBracedLogicalIds(changes.summarizeManagedPolicies())));
this.print(formatTable(this.deepSubstituteBracedLogicalIds(changes.summarizeManagedPolicies()), this.stream.columns));
}
}

public formatSecurityGroupChanges(changes: SecurityGroupChanges) {
if (!changes.hasChanges) { return; }

this.printSectionHeader('Security Group Changes');
this.print(renderTable(this.deepSubstituteBracedLogicalIds(changes.summarize())));
this.print(formatTable(this.deepSubstituteBracedLogicalIds(changes.summarize()), this.stream.columns));
}

public deepSubstituteBracedLogicalIds(rows: string[][]): string[][] {
Expand All @@ -382,47 +382,6 @@ class Formatter {
}
}

/**
* Render a two-dimensional array to a visually attractive table
*
* First row is considered the table header.
*/
function renderTable(cells: string[][]): string {
const head = cells.splice(0, 1)[0];

const table = new Table({ head, style: { head: [] } });
table.push(...cells);
return stripHorizontalLines(table.toString()).trimRight();
}

/**
* Strip horizontal lines in the table rendering if the second-column values are the same
*
* We couldn't find a table library that BOTH does newlines-in-cells correctly AND
* has an option to enable/disable separator lines on a per-row basis. So we're
* going to do some character post-processing on the table instead.
*/
function stripHorizontalLines(tableRendering: string) {
const lines = tableRendering.split('\n');

let i = 3;
while (i < lines.length - 3) {
if (secondColumnValue(lines[i]) === secondColumnValue(lines[i + 2])) {
lines.splice(i + 1, 1);
i += 1;
} else {
i += 2;
}
}

return lines.join('\n');

function secondColumnValue(line: string) {
const cols = colors.stripColors(line).split('│').filter(x => x !== '');
return cols[1];
}
}

/**
* A patch as returned by ``diff.structuredPatch``.
*/
Expand Down
6 changes: 4 additions & 2 deletions packages/@aws-cdk/cloudformation-diff/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,16 @@
"dependencies": {
"@aws-cdk/cfnspec": "^0.22.0",
"@aws-cdk/cx-api": "^0.22.0",
"cli-table": "^0.3.1",
"string-width": "^2.1.1",
"table": "^5.2.0",
"colors": "^1.2.1",
"diff": "^4.0.1",
"fast-deep-equal": "^2.0.1",
"source-map-support": "^0.5.6"
},
"devDependencies": {
"@types/cli-table": "^0.3.0",
"@types/table": "^4.0.5",
"@types/string-width": "^2.0.0",
"cdk-build-tools": "^0.22.0",
"fast-check": "^1.8.0",
"pkglint": "^0.22.0"
Expand Down

0 comments on commit f0975c1

Please sign in to comment.