Skip to content

Commit

Permalink
Rewrite table-cell-padding with several fixes
Browse files Browse the repository at this point in the history 
Basically, the rule is rewritten as the previous one did not work
as expected.  The behaviour now is what previously was intended.
More cells are checked.  When compact, everything but the biggest
cell is ignored (as shorter cells may be padded for aligning fences).
Empty body- and heading cells are also handled properly.

Finally, there’s a better message when `'padded'`, if the biggest
cell is padded more than required.

Closes GH-108.
  • Loading branch information
@wooorm
wooorm committed Dec 12, 2016
1 parent ce5995a commit e340c2c
Show file tree
Hide file tree
Showing 2 changed files with 177 additions and 100 deletions.
50 changes: 44 additions & 6 deletions doc/rules.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2464,6 +2464,20 @@ When this rule is `'padded'`, the following file
| Alpha | Bravo |
```

When this rule is `'padded'`, the following file
`invalid.md` is **not** ok:

```markdown
| A | B |
| :----|----: |
| Alpha|Bravo |
```

```text
3:8: Cell should be padded
3:9: Cell should be padded
```

When this rule is `'compact'`, the following file
`valid.md` is ok:

Expand All @@ -2475,6 +2489,19 @@ When this rule is `'compact'`, the following file
|Alpha|Bravo|
```

When this rule is `'compact'`, the following file
`invalid.md` is **not** ok:

```markdown
|A | B|
|:----|-----:|
|Alpha|Bravo |
```

```text
3:13: Cell should be compact
```

When this rule is turned on, the following file
`invalid.md` is **not** ok:

Expand All @@ -2487,22 +2514,33 @@ When this rule is turned on, the following file
```

```text
3:5: Cell should be padded, isn’t
3:9: Cell should be padded, isn’t
3:16: Cell should be padded, isn’t
5:5: Cell should be padded with 1 space, not 3
5:10: Cell should be padded
5:17: Cell should be padded
```

When this rule is turned on, the following file
`empty.md` is ok:
`empty-heading.md` is ok:

```markdown
<!-- Empty cells are always OK. -->
<!-- Empty heading cells are always OK. -->

| Alpha | |
| | Alpha |
| ----- | ------- |
| Bravo | Charlie |
```

When this rule is turned on, the following file
`empty-body.md` is ok:

```markdown
<!-- Empty body cells are always OK. -->

| Alpha | Bravo |
| ------- | ------- |
| Charlie | |
```

When `'invalid'` is passed in, the following error is given:

```text
Expand Down
227 changes: 133 additions & 94 deletions packages/remark-lint/lib/rules/table-cell-padding.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,161 +39,200 @@
*
* @example {"name": "invalid.md", "label": "output"}
*
* 3:5: Cell should be padded, isn’t
* 3:9: Cell should be padded, isn’t
* 3:16: Cell should be padded, isn’t
* 5:5: Cell should be padded with 1 space, not 3
* 5:10: Cell should be padded
* 5:17: Cell should be padded
*
* @example {"name": "invalid.md", "label": "input", "setting": "padded"}
*
* | A | B |
* | :----|----: |
* | Alpha|Bravo |
*
* @example {"name": "invalid.md", "label": "output", "setting": "padded"}
*
* 3:8: Cell should be padded
* 3:9: Cell should be padded
*
* @example {"name": "invalid.md", "label": "input", "setting": "compact"}
*
* |A | B|
* |:----|-----:|
* |Alpha|Bravo |
*
* @example {"name": "invalid.md", "label": "output", "setting": "compact"}
*
* 3:13: Cell should be compact
*
* @example {"name": "invalid.md", "label": "output", "setting": "invalid", "config": {"positionless": true}}
*
* 1:1: Invalid table-cell-padding style `invalid`
*
* @example {"name": "empty.md"}
* @example {"name": "empty-heading.md"}
*
* <!-- Empty cells are always OK. -->
* <!-- Empty heading cells are always OK. -->
*
* | Alpha | |
* | | Alpha |
* | ----- | ------- |
* | Bravo | Charlie |
*
* @example {"name": "empty-body.md"}
*
* <!-- Empty body cells are always OK. -->
*
* | Alpha | Bravo |
* | ------- | ------- |
* | Charlie | |
*/

'use strict';

/* eslint-disable max-params */

/* Dependencies. */
var visit = require('unist-util-visit');
var position = require('unist-util-position');
var generated = require('unist-util-generated');

/* Expose. */
module.exports = tableCellPadding;

/* Methods. */
var start = position.start;
var end = position.end;

/* Valid styles. */
var STYLES = {
null: true,
padded: true,
compact: true
};

/**
* Warn when table cells are incorrectly padded.
*
* @param {Node} ast - Root node.
* @param {File} file - Virtual file.
* @param {string?} preferred - Either `padded` (for
* at least a space), `compact` (for no spaces when
* possible), or `consistent`, which defaults to the
* first found style.
*/
function tableCellPadding(ast, file, preferred) {
function tableCellPadding(tree, file, preferred) {
preferred = typeof preferred !== 'string' || preferred === 'consistent' ? null : preferred;

if (STYLES[preferred] !== true) {
file.fail('Invalid table-cell-padding style `' + preferred + '`');
}

visit(ast, 'table', function (node) {
var children = node.children;
var contents = file.toString();
visit(tree, 'table', visitor);

return;

function visitor(node) {
var rows = node.children;
var contents = String(file);
var starts = [];
var ends = [];
var cells;
var locations;
var positions;
var cells = [];
var style;
var type;
var warning;
var sizes;

if (generated(node)) {
return;
}

/**
* Check a fence. Checks both its initial spacing
* (between a cell and the fence), and its final
* spacing (between the fence and the next cell).
*
* @param {number} initial - Starting index.
* @param {number} final - Closing index.
* @param {Node} cell - Table cell.
* @param {Node?} next - Following cell.
* @param {number} index - Position of `cell` in
* its parent.
*/
function check(initial, final, cell, next, index) {
var fence = contents.slice(initial, final);
var pos = fence.indexOf('|');
rows.forEach(eachRow);

if (
cell &&
pos !== -1 &&
(ends[index] === undefined || pos < ends[index])
) {
ends[index] = pos;
}
sizes = inferSizes(node);

if (next && pos !== -1) {
pos = fence.length - pos - 1;
if (preferred === 'padded') {
style = 1;
} else if (preferred === 'compact') {
style = 0;
} else {
style = null;
starts.concat(ends).some(inferStyle);
}

if (starts[index + 1] === undefined || pos < starts[index + 1]) {
starts[index + 1] = pos;
}
cells.forEach(checkCell);

return;

function eachRow(row) {
var children = row.children;

check(start(row).offset, start(children[0]).offset, null, children[0]);
ends.pop(); /* Ignore end before row. */

children.forEach(eachCell);
starts.pop(); /* Ignore start after row */

function eachCell(cell, index) {
var next = children[index + 1] || null;
check(end(cell).offset, start(next).offset || end(row).offset, cell, next);
cells.push(cell);
}
}

children.forEach(function (row) {
var cells = row.children;
function inferStyle(pos) {
if (pos === undefined) {
return false;
}

check(start(row).offset, start(cells[0]).offset, null, cells[0], -1);
style = Math.min(pos, 1);
return true;
}

cells.forEach(function (cell, index) {
var next = cells[index + 1] || null;
var final = start(next).offset || end(row).offset;
function check(initial, final, prev, next) {
var fence = contents.slice(initial, final);
var pos = fence.indexOf('|');

check(end(cell).offset, final, cell, next, index);
});
});
ends.push(prev && pos !== -1 && prev.children.length !== 0 ? pos : undefined);
starts.push(next && next.children.length !== 0 ? fence.length - pos - 1 : undefined);
}

positions = starts.concat(ends);
function checkCell(cell, index) {
/* Ignore, when compact, every cell except the biggest in the column. */
if (style === 0 && size(cell) < sizes[index % sizes.length]) {
return;
}

if (preferred === 'padded') {
style = 1;
} else if (preferred === 'compact') {
style = 0;
} else {
style = null;
checkSide('start', cell, starts[index], index);
checkSide('end', cell, ends[index]);
}

positions.some(function (pos) {
/* `some` skips non-existant indices, so
* there's no need to check for `!isNaN`. */
style = Math.min(pos, 1);
function checkSide(side, cell, spacing, index) {
var message;

return true;
});
if (spacing === undefined || spacing === style) {
return;
}

message = 'Cell should be ';

if (style === 0) {
message += 'compact';
} else {
message += 'padded';

if (spacing > style) {
message += ' with 1 space, not ' + spacing;

/* May be right or center aligned. */
if (size(cell) < sizes[index % sizes.length]) {
return;
}
}
}

file.message(message, cell.position[side]);
}
}
}

cells = children[0].children;
function inferSizes(tree) {
var sizes = Array(tree.align.length);

locations = cells.map(function (cell) {
return start(cell);
}).concat(cells.map(function (cell) {
return end(cell);
}));
tree.children.forEach(row);

cells = cells.concat(cells);
type = style === 1 ? 'padded' : 'compact';
warning = 'Cell should be ' + type + ', isn’t';
return sizes;

positions.forEach(function (diff, index) {
var cell = cells[index];
function row(node) {
node.children.forEach(cell);
}

if (cell && cell.children.length !== 0 && diff !== style && diff !== undefined && diff !== null) {
file.message(warning, locations[index]);
}
});
});
function cell(node, index) {
sizes[index] = Math.max(sizes[index] || 0, size(node));
}
}

function size(node) {
return end(node).offset - start(node).offset;
}

0 comments on commit e340c2c

Please sign in to comment.