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

Parse sub-lists indented with tabs #347

Closed
wants to merge 6 commits into from
Closed

Parse sub-lists indented with tabs #347

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ac7b893
remark-parse parse sub-lists indented with tabs
transitive-bullshit Aug 8, 2018
5fd0e66
remark-parse remove invalid block from remove-indentation
transitive-bullshit Aug 8, 2018
c920366
fix lint
transitive-bullshit Aug 10, 2018
5c658f9
pr feedback; regenerate-fixtures
transitive-bullshit Aug 19, 2018
244b10f
remove extra deep
transitive-bullshit Aug 19, 2018
3ae5e78
fix lint
transitive-bullshit Aug 19, 2018
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 package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"tape": "^4.5.1",
"uglify-js": "^3.0.28",
"unist-builder": "^1.0.2",
"unist-util-inspect": "^4.1.3",
Copy link
Member

Choose a reason for hiding this comment

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

Is this needed?

Copy link
Author

Choose a reason for hiding this comment

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

removed :)

"unist-util-remove-position": "^1.1.0",
"unist-util-visit": "^1.1.0",
"vfile": "^2.0.0",
Expand Down
5 changes: 3 additions & 2 deletions packages/remark-parse/lib/tokenize/list.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,8 @@ function normalListItem(ctx, value, position) {

lines = value.split(C_NEWLINE);

trimmedLines = removeIndent(value, getIndent(max).indent).split(C_NEWLINE);
var maxIndent = getIndent(max);
Copy link
Member

Choose a reason for hiding this comment

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

could you move the var declaration to the top? So var maxIndent above, and maxIndent = getIndent(max) here?

trimmedLines = removeIndent(value, maxIndent.indent).split(C_NEWLINE);

/* We replaced the initial bullet with something
* else above, which was used to trick
Expand Down Expand Up @@ -467,7 +468,7 @@ function normalListItem(ctx, value, position) {
$2 = C_SPACE + $2;
}

max = $1 + repeat(C_SPACE, $2.length) + $3;
max = $1 + repeat(C_SPACE, Math.max($2.length, TAB_SIZE)) + $3;
Copy link
Author

@transitive-bullshit transitive-bullshit Aug 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

This is necessary because we want to trim ensuing lines if they contain at least a \t, but if the first line doeesn't contain a \t, we won't properly trim them here.

I've looked through the changed (failing) test cases that this edit introduces, and afaict, their output fixtures were less correct before this change.


return max + rest;
}
Expand Down
14 changes: 1 addition & 13 deletions packages/remark-parse/lib/util/remove-indentation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ module.exports = indentation;

var C_SPACE = ' ';
var C_NEWLINE = '\n';
var C_TAB = '\t';

/* Remove the minimum indent from every line in `value`.
* Supports both tab, spaced, and mixed indentation (as
Expand All @@ -21,7 +20,6 @@ function indentation(value, maximum) {
var index;
var indentation;
var stops;
var padding;

values.unshift(repeat(C_SPACE, maximum) + '!');

Expand Down Expand Up @@ -56,17 +54,7 @@ function indentation(value, maximum) {
index--;
}

if (
trim(values[position]).length !== 0 &&
minIndent &&
index !== minIndent
) {
padding = C_TAB;
} else {
padding = '';
}
Copy link
Author

@transitive-bullshit transitive-bullshit Aug 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

This logic for adding padding in a function that is supposed to be removing padding is definitely incorrect. I've checked the failing test cases before and after this change, and this code block does more harm than good. There is only one test case whose output changes after making this change, and the previous fixture expected it to introduce a phantom \t into the output text paragraph. By removing this block, the phantom \t is removed and the test output is correct afaict.


values[position] = padding + values[position].slice(
values[position] = values[position].slice(
index in stops ? stops[index] + 1 : 0
);
}
Expand Down