Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Fix: improve comment indentation (fixes #514) #676

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.

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

Merged
michalsnik merged 3 commits into master from issue514
Nov 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
108 changes: 71 additions & 37 deletions lib/utils/indent-common.js
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const KNOWN_NODES = new Set(['ArrayExpression', 'ArrayPattern', 'ArrowFunctionEx
const LT_CHAR = /[\r\n\u2028\u2029]/
const LINES = /[^\r\n\u2028\u2029]+(?:$|\r\n|[\r\n\u2028\u2029])/g
const BLOCK_COMMENT_PREFIX = /^\s*\*/
const ITERATION_OPTS = Object.freeze({ includeComments: true, filter: isNotWhitespace })

/**
* Normalize options.
Expand Down Expand Up @@ -194,6 +195,15 @@ function isNotComment (token) {
return token != null && token.type !== 'Block' && token.type !== 'Line' && token.type !== 'Shebang' && !token.type.endsWith('Comment')
}

/**
* Check whether the given node is not an empty text node.
* @param {Node} node The node to check.
* @returns {boolean} `false` if the token is empty text node.
*/
function isNotEmptyTextNode (node) {
return !(node.type === 'VText' && node.value.trim() === '')
}

/**
* Get the last element.
* @param {Array} xs The array to get the last element.
Expand Down Expand Up @@ -295,7 +305,7 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
* @param {Token} token The token to set.
* @returns {void}
*/
function setBaseline (token, hardTabAdditional) {
function setBaseline (token) {
const offsetInfo = offsets.get(token)
if (offsetInfo != null) {
offsetInfo.baseline = true
Expand Down Expand Up @@ -353,17 +363,21 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
* The first node is offsetted from the given left token.
* Rest nodes are adjusted to the first node.
* @param {Node[]} nodeList The node to process.
* @param {Node|null} leftToken The left parenthesis token.
* @param {Node|null} rightToken The right parenthesis token.
* @param {Node|Token|null} left The left parenthesis token.
* @param {Node|Token|null} right The right parenthesis token.
* @param {number} offset The offset to set.
* @param {Node} [alignVertically=true] The flag to align vertically. If `false`, this doesn't align vertically even if the first node is not at beginning of line.
* @param {boolean} [alignVertically=true] The flag to align vertically. If `false`, this doesn't align vertically even if the first node is not at beginning of line.
* @returns {void}
*/
function processNodeList (nodeList, leftToken, rightToken, offset, alignVertically) {
function processNodeList (nodeList, left, right, offset, alignVertically) {
let t
const leftToken = (left && tokenStore.getFirstToken(left)) || left
const rightToken = (right && tokenStore.getFirstToken(right)) || right

if (nodeList.length >= 1) {
let lastToken = leftToken
let baseToken = null
let lastToken = left
const alignTokensBeforeBaseToken = []
const alignTokens = []

for (let i = 0; i < nodeList.length; ++i) {
Expand All @@ -374,30 +388,50 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
}
const elementTokens = getFirstAndLastTokens(node, lastToken != null ? lastToken.range[1] : 0)

// Collect related tokens.
// Commas between this and the previous, and the first token of this node.
// Collect comma/comment tokens between the last token of the previous node and the first token of this node.
if (lastToken != null) {
t = lastToken
while ((t = tokenStore.getTokenAfter(t)) != null && t.range[1] <= elementTokens.firstToken.range[0]) {
alignTokens.push(t)
while (
(t = tokenStore.getTokenAfter(t, ITERATION_OPTS)) != null &&
t.range[1] <= elementTokens.firstToken.range[0]
) {
if (baseToken == null) {
alignTokensBeforeBaseToken.push(t)
} else {
alignTokens.push(t)
}
}
}
alignTokens.push(elementTokens.firstToken)

// Save the last token to find tokens between the next token.
if (baseToken == null) {
baseToken = elementTokens.firstToken
} else {
alignTokens.push(elementTokens.firstToken)
}

// Save the last token to find tokens between this node and the next node.
lastToken = elementTokens.lastToken
}

// Check trailing commas.
// Check trailing commas and comments.
if (rightToken != null && lastToken != null) {
t = lastToken
while ((t = tokenStore.getTokenAfter(t)) != null && t.range[1] <= rightToken.range[0]) {
alignTokens.push(t)
while (
(t = tokenStore.getTokenAfter(t, ITERATION_OPTS)) != null &&
t.range[1] <= rightToken.range[0]
) {
if (baseToken == null) {
alignTokensBeforeBaseToken.push(t)
} else {
alignTokens.push(t)
}
}
}

// Set offsets.
const baseToken = alignTokens.shift()
if (leftToken != null) {
setOffset(alignTokensBeforeBaseToken, offset, leftToken)
}
if (baseToken != null) {
// Set offset to the first token.
if (leftToken != null) {
Expand All @@ -409,7 +443,7 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
setBaseline(baseToken)
}

if (alignVertically === false) {
if (alignVertically === false && leftToken != null) {
// Align tokens relatively to the left token.
setOffset(alignTokens, offset, leftToken)
} else {
Expand Down Expand Up @@ -657,10 +691,10 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
* Validate the given token with the pre-calculated expected indentation.
* @param {Token} token The token to validate.
* @param {number} expectedIndent The expected indentation.
* @param {number|undefined} optionalExpectedIndent The optional expected indentation.
* @param {number[]|undefined} optionalExpectedIndents The optional expected indentation.
* @returns {void}
*/
function validateCore (token, expectedIndent, optionalExpectedIndent) {
function validateCore (token, expectedIndent, optionalExpectedIndents) {
const line = token.loc.start.line
const indentText = getIndentText(token)

Expand Down Expand Up @@ -692,7 +726,7 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
}
}

if (actualIndent !== expectedIndent && (optionalExpectedIndent === undefined || actualIndent !== optionalExpectedIndent)) {
if (actualIndent !== expectedIndent && (optionalExpectedIndents == null || !optionalExpectedIndents.includes(actualIndent))) {
context.report({
loc: {
start: { line, column: 0 },
Expand All @@ -716,7 +750,7 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
* @param {Token|null} nextToken The next token of comments.
* @param {number|undefined} nextExpectedIndent The expected indent of the next token.
* @param {number|undefined} lastExpectedIndent The expected indent of the last token.
* @returns {{primary:number|undefined,secondary:number|undefined}}
* @returns {number[]}
*/
function getCommentExpectedIndents (nextToken, nextExpectedIndent, lastExpectedIndent) {
if (typeof lastExpectedIndent === 'number' && isClosingToken(nextToken)) {
Expand All @@ -725,26 +759,23 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
// <div>
// <!-- comment -->
// </div>
return {
primary: nextExpectedIndent + options.indentSize,
secondary: undefined
}
return [nextExpectedIndent + options.indentSize, nextExpectedIndent]
}

// For last comment. E.g.,
// <div>
// <div></div>
// <!-- comment -->
// </div>
return { primary: lastExpectedIndent, secondary: nextExpectedIndent }
return [lastExpectedIndent, nextExpectedIndent]
}

// Adjust to next normally. E.g.,
// <div>
// <!-- comment -->
// <div></div>
// </div>
return { primary: nextExpectedIndent, secondary: undefined }
return [nextExpectedIndent]
}

/**
Expand Down Expand Up @@ -815,11 +846,17 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
// It allows the same indent level with the previous line.
const lastOffsetInfo = offsets.get(lastToken)
const lastExpectedIndent = lastOffsetInfo && lastOffsetInfo.expectedIndent
const commentExpectedIndents = getCommentExpectedIndents(firstToken, expectedIndent, lastExpectedIndent)
const commentOptionalExpectedIndents = getCommentExpectedIndents(firstToken, expectedIndent, lastExpectedIndent)

// Validate.
for (const comment of comments) {
validateCore(comment, commentExpectedIndents.primary, commentExpectedIndents.secondary)
const commentExpectedIndents = getExpectedIndents([comment])
const commentExpectedIndent =
commentExpectedIndents
? commentExpectedIndents.expectedIndent
: commentOptionalExpectedIndents[0]

validateCore(comment, commentExpectedIndent, commentOptionalExpectedIndents)
}
validateCore(firstToken, expectedIndent)
}
Expand All @@ -844,16 +881,14 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
},

VElement (node) {
const startTagToken = tokenStore.getFirstToken(node)
const endTagToken = node.endTag && tokenStore.getFirstToken(node.endTag)

if (node.name !== 'pre') {
const childTokens = node.children.map(n => tokenStore.getFirstToken(n))
setOffset(childTokens, 1, startTagToken)
processNodeList(node.children.filter(isNotEmptyTextNode), node.startTag, node.endTag, 1)
} else {
const startTagToken = tokenStore.getFirstToken(node)
const endTagToken = node.endTag && tokenStore.getFirstToken(node.endTag)
setOffset(endTagToken, 0, startTagToken)
setPreformattedTokens(node)
}
setOffset(endTagToken, 0, startTagToken)
},

VEndTag (node) {
Expand Down Expand Up @@ -1116,7 +1151,6 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti

setOffset(leftParenToken, 1, forToken)
processNodeList([node.init, node.test, node.update], leftParenToken, rightParenToken, 1)
setOffset(rightParenToken, 0, leftParenToken)
processMaybeBlock(node.body, forToken)
},

Expand Down Expand Up @@ -1544,7 +1578,7 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
let lastValidatedToken = null

// Validate indentation of tokens.
for (const token of tokenStore.getTokens(node, { includeComments: true, filter: isNotWhitespace })) {
for (const token of tokenStore.getTokens(node, ITERATION_OPTS)) {
if (tokensOnSameLine.length === 0 || tokensOnSameLine[0].loc.start.line === token.loc.start.line) {
// This is on the same line (or the first token).
tokensOnSameLine.push(token)
Expand Down
8 changes: 8 additions & 0 deletions tests/fixtures/html-indent/issue514.vue
View file Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<!--{}-->
<template>
<div>
<input type="text"
value="foo">
<!-- comment -->
</div>
</template>

AltStyle によって変換されたページ (->オリジナル) /