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

Handle Windows newlines gracefully #205

Closed
g-andrade opened this issue Aug 30, 2021 · 3 comments · Fixed by #206
Closed

Handle Windows newlines gracefully #205

g-andrade opened this issue Aug 30, 2021 · 3 comments · Fixed by #206

Comments

@g-andrade
Copy link
Contributor

Bug Description

Recently I tried to run CI for my lib on Windows.

All the tests passed (surprisingly!) but Elvis (invoked through rebar3_lint) complained a lot about trailing spaces and operator spaces. Like, for almost every line of code.

So, what's happening is: git can be configured to automatically convert line endings into whatever the OS's native ones are.

And that's what it does by default on the Windows image I used, apparently.

However, elvis_core only knows how to split lines using \n (e.g. here, which leaves all of those extra \r around (and they qualify as whitespace.)

To Reproduce

Now that's tricky 🤔 I don't have Windows myself.

But it boils down to:

  1. clone a rebar3 project which integrates rebar3_lint on Windows, and
  2. run rebar3 lint.

Alternatively, leveraging GitHub Actions for CI'ing on Windows should be enough to confirm the problem.

Expected Behavior

I expected that any newline-affected rules (e.g. no_trailing_whitespace) would take into account Windows newline endings gracefully (\r\n rather than just \n.)

rebar3 Logs

Here it is (let me know if more details are needed.)

Additional Context

  • OS: Microsoft Windows Server 2019 (10.0.17763)
  • Erlang version: OTP 24.0.5
  • rebar3 version: 3.16.1
  • rebar3_lint version: 0.5.0 (which imports elvis_core 1.2.0)
@g-andrade
Copy link
Contributor Author

g-andrade commented Aug 30, 2021

By the way: in the process of investigating the issue I pretty much fixed it, so let me know and I can open a PR (which I didn't do already because at some point I read CONTRIBUTING.md 😄 )

@g-andrade
Copy link
Contributor Author

Here's the fix and here's confirmation that it worked.

If you agree with the solution, I'll open a PR.

If not, I'm open to exploring other options (and I'll understand if you don't even consider this a real problem, given how few people are actually using Windows to develop Erlang.)

@elbrujohalcon
Copy link
Member

elbrujohalcon commented Aug 31, 2021

@g-andrade Great catch! Amazing description! Incredibly thorough explanation! And perfect fix!

I would ❤️ to have that as a PR 🙏🏻

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 a pull request may close this issue.

2 participants