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.

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

docs: remove wrong examples in README.md(check-alignment) #340

Closed
wants to merge 1 commit into from
Closed

docs: remove wrong examples in README.md(check-alignment) #340

wants to merge 1 commit into from

Conversation

yeonjuan
Copy link
Contributor

The two examples you explained in README.md(check-alignment) are wrong I think, you mentioned 'thees are wrong', but in my opinion it is right.
When I run eslint with plugin these two didn't warn.

@brettz9
Copy link
Collaborator

brettz9 commented Jul 15, 2019

The examples are built from the test files, and the test files do not have this issue.

The issue is within src/bin/assertions.js, specifically the function trimCode which trims initial whitespace and uses the last line of code to determine a fixed amount of whitespace to remove from each line besides the first (so as to remove the extra whitespace typically used to offset the code in the test samples).

So if you could file an issue rather than this specific PR (or alter the PR to fix the function), we can track the need for fixing the trimCode function.

@brettz9
Copy link
Collaborator

brettz9 commented Jul 15, 2019

It seems your second example wasn't fixed by the #342 fix though. Could you take a look at how we might fix that, @yeonjuan ?

@yeonjuan
Copy link
Contributor Author

yeonjuan commented Jul 17, 2019

@brettz9
The Solution is just change

  lines = lines.map((line, index) => {
    const lineIndentSize = firstIndentSize !== 0 || index === 0
      ? Math.min(firstIndentSize, lastIndentSize)
      : lastIndentSize;

    return line.slice(lineIndentSize);
  });

But In ./test/rules/assertions/ there are some incorrect exemple with 'block alignment rule' .
ex) './test/rules/assertions/requireReturns.js' line 174 ~ 176 ( need one space more)

So If you agree, I'll fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants