Skip to content

Allow trailing_whitespace_linter to skip empty lines #165

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

Closed
wants to merge 2 commits into from

Conversation

flying-sheep
Copy link

Also included backwards compatible code for people putting trailing_whitespace_linter in their config

@codecov-io
Copy link

Current coverage is 81.25% (diff: 88.88%)

Merging #165 into master will decrease coverage by 0.13%

@@             master       #165   diff @@
==========================================
  Files            30         30          
  Lines          1682       1686     +4   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1369       1370     +1   
- Misses          313        316     +3   
  Partials          0          0          

Powered by Codecov. Last update e6245de...8669f8c

@jimhester
Copy link
Member

Can you explain the reason you want this? Extra whitespace in "empty" lines is just as useless as extra whitespace in any other line and should be linted and removed.

@flying-sheep
Copy link
Author

i like to be able to put my cursor there and type instead of having to adjust indentation first. this requires empty lines to be indented just as much as surrounding lines.

i plan to add a linter that checks for uneven indentation. it will get an option to also check empty lines, which would collide with a trailing_whitespace_linter that doesn’t have that option.

@naught101
Copy link

I think this is a bad idea. A good editor should automatically indent a new line to the correct depth without needing to leave white space in empty lines. I know that vim does it (e.g. indents an extra 4 spaces after braces, and indents up to the parenthesis when splitting function calls over multiple lines). Is there any case other than these where pre-indentation is helpful?

@flying-sheep
Copy link
Author

It's simply a matter of preference. You don't need to think it's a good idea as long as it's optional.

@AshesITR
Copy link
Collaborator

AshesITR commented Apr 11, 2020

Note that RStudio is one of those editors that will happily keep "empty" lines padded with spaces when they are indented. Since that's a very common IDE for R development, I'd also like this feature.

Just type something like this in a new RStudio window, save & lint:

for (i in 1:10) {
  x <- 2 * i
  # Hit Enter 2x:

  print(i)
}

Unless the option Code > Saving > Strip trailing horizontal whitespace when saving is active, it will produce a lint.
AFAIK the option is off by default.

@MichaelChirico
Copy link
Collaborator

Anyone still interested in keeping this in lintr?

@AshesITR
Copy link
Collaborator

Is this still a default behaviour of RStudio?
If so, I think it's worth adding the option to ignore pure-whitespace lines.

@AshesITR
Copy link
Collaborator

AshesITR commented Apr 2, 2022

NB currently, completely blank lines aren't caught because the linter operates on expressions and not globally, so only lines with expressions are considered for linting.

@AshesITR
Copy link
Collaborator

AshesITR commented Apr 2, 2022

Superseded by #1044

@AshesITR AshesITR closed this Apr 2, 2022
AshesITR added a commit that referenced this pull request Apr 4, 2022
* add option to ignore blank lines to trailing_whitespace_linter()

supersedes #165

* update issue and pr numbers

* grammar

* whoops

* fix in Rd

* new unit for new case

* un-break test

Co-authored-by: Michael Chirico <michaelchirico4@gmail.com>
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.

6 participants