-
-
Notifications
You must be signed in to change notification settings - Fork 749
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
Conflict between MD007/ul-indent with start_indented and MD027/no-multiple-space-blockquotes #1473
Comments
Thank you for the thorough issue! Here's a simplified demonstration of the behavior you describe: https://dlaa.me/markdownlint/#%25m%23%20Issue%201473%0A%0A%3E%20%20%20%2B%20Item%0A%3E%20%20%20%2B%20Item%0A%3E%20%20%20%20%20%2B%20Item%0A%0A%3C!--%20markdownlint-configure-file%20%7B%0A%20%20%22ul-indent%22%3A%20%7B%0A%20%20%20%20%22indent%22%3A%202%2C%0A%20%20%20%20%22start_indented%22%3A%20true%0A%20%20%7D%2C%0A%20%20%20%20%22no-multiple-space-blockquote%22%3A%20true%0A%7D%20--%3E%0A The current behavior is inconsistent, but the old behavior was not what you want, either. :) While I believe this worked as you want in older versions, that was luck. In general, rules are independent and do NOT know about each other. Specifically, they are NOT able to access the configuration for other rules. Because your scenario only arises when MD007 is customized, the lack of MD027's ability to perceive that proves it wasn't behaving as you wish intentionally (you can confirm by looking for Based on code inspection, it seems the current behavior of MD027 handling indented lists differently is because it keys off the micromark
I can fix the inconsistency with blockquoted indented lists, but your scenario will still report violations because MD027 won't know to respect the custom configuration you provided for MD007. In cases like this, I typically advise disabling one of the overlapping rules. I've only superficially looked at this so far, and I'm open to clever solutions about how to make your scenario work without disabling a rule. |
I see, thank you for the explanation, it makes sense that they're not aware of each other. I believe the old behavior happened to work because of how it used the function MD027(params, onError) {
const { tokens } = params.parsers.micromark;
for (const token of filterByTypesCached([ "linePrefix" ])) {
const parent = token.parent;
const codeIndented = parent?.type === "codeIndented";
const siblings = parent?.children || tokens;
const currentTokenIndex = siblings.indexOf(token);
const previousSiblingType = siblings[currentTokenIndex - 1]?.type;
const nextSiblingType = siblings[currentTokenIndex + 1]?.type;
const listTypes = [ "listItemPrefix", "listOrdered", "listUnordered" ];
if (
!codeIndented &&
previousSiblingType === "blockQuotePrefix" &&
!listTypes.includes(nextSiblingType)
) {
const { startColumn, startLine, text } = token;
const { length } = text;
const line = params.lines[startLine - 1];
addErrorContext(
onError,
startLine,
line,
undefined,
undefined,
[ startColumn, length ],
{
"editColumn": startColumn,
"deleteCount": length
}
);
}
}
} Meanwhile, we'll disable the no-multiple-space-blockquote rule as you suggested. Thank you. |
I need to play around with this myself before I have an opinion about what changes to make. I should have an opportunity to do that this weekend. |
Just FYI, I've looked at this a bit and I'm finding more ways I'm hesitant to try to do too much self-parsing here and therefore inclined to think the current implementation that uses |
I'm changing this from "bug" to "enhancement" because:
In a couple of minutes, I'll commit some test cases I put together for future reference. |
Would it be reasonable to add a boolean option like If acceptable, we would be able to just stop checking multiple spaces in blockquotes for all blockquote lines with list elements, and then safely apply indentation rules specific to lists. This could prevent the conflict scenario. |
I think that would be reasonable, yes. :) I can't, on brief reflection, think of another situation where this would be a problem. For consistency, I think the parameter should be I expect to get to this in a few days. Thank you! |
That's great, thank you! |
What is happening?
Markdownlint raises an error for
ul-indent
when a list is not indented inside a blockquote andstart_indented
is set totrue
; when the indentation is fixed, the ruleno-multiple-space-blockquote
raises an error indicating that a blockquote should not have multiple spaces.Why is it happening?
When the
start_indented
option oful-indent
rule istrue
, it demands a specific starting indentation for a list, anywhere they appear. When inside a blockquote with the ruleno-multiple-space-blockquote
enabled, both rules try to be applied, negating each other.Expected behavior
ul-indent
withindent
bigger than zero andstart_indented
astrue
should work normally inside blockquotes, as it used to in previous versions.This rule already existed and was used before, so why did it only started raising an error now?
ul-indent
withstart_indented
option astrue
, andno-multiple-space-blockquote
have been conflicting in every version ever since, which includes the current markdownlint (0.37.4) and, as a consequence, markdownlint-cli2 (0.17.2)What have I tried?
After finding the complete rewriting of rule
MD027
in the version which the error starts, I reverted the implementation ofMD027/no-multiple-space-blockquote
locally to its previous version and the conflict stopped happening, I was then able to useMD007/ul-indent
normally withstart_indented
set totrue
inside blockquotes without any lint errors.Goal of this issue
Bring back to current implementation the behavior where lists could adhere to
MD007/ul-indent
withstart_indented
inside blockquotes without conflicting withMD027/no-multiple-space-blockquote
.Setup:
Same results were obtained on node 23.13.1 (current lts) and npm 11.0.0 (current latest)
Steps to reproduce:
I've set up a repository here (https://github.com/MatheusBaldi/markdownlint-md027-issue-mre) for convenience. You can clone it,
npm install
, and then runnpx [email protected] .
.If you want to set up your own project, here are the steps:
npx [email protected] .
files:
.markdownlint.json:
examples.md:
The text was updated successfully, but these errors were encountered: