extend infix_spaces_linter to be more flexible & correct #931
Conversation
| "<-", | ||
| ":=", | ||
| "<<-", | ||
| "<", | ||
| ">", | ||
| "->", | ||
| "->>", |
There was a problem hiding this comment.
I just noticed that the test suite tests 1 <- 2 etc.
IMO we should add sensible test cases for the assignment operators.
There was a problem hiding this comment.
I copy-pasted the tests for the deleted assignment_spaces_linter here. I also removed a bunch of them that were throwing lints for using >1 space, because the style guide says that's OK if it's used to improve alignment and our logic is (and I think should continue to be) not sophisticated enough to detect alignment issues:
https://style.tidyverse.org/syntax.html#extra-spaces
Adding extra spaces is ok if it improves alignment of
=or<-.
There was a problem hiding this comment.
Since @f-ritter's original use-case was disallowing these, I think we should also support that rule via an argument (allow_multiple_spaces = TRUE). WDYT?
There was a problem hiding this comment.
let's file it as a follow-up to keep this PR cleaner
There was a problem hiding this comment.
Okay, should be a lot easier if we convert the linter to XPath at the same time (requires something like following-sibling::*[@col1 = preceding-sibling::*/@col2] vs. > preceding-sibling::*/@col2 iinm) #940
|
Looks like we were affected by https://www.githubstatus.com/incidents/fpk08rxnqjz2 but it's not clear how to trigger a restart? |
|
I tried to "trick" GHA to restart by switching the base branch twice, but it killed your approval. and also didn't start the actions 🙈 |
|
I removed the minor |
|
── Failure (test-linter_tags.R:26:3): available_linters matches the set of linters available from lintr ── That's a fast ROI on the new test 😁 |
|
indeed... bad merge i think. restored the gitignore edit while we're at it |
|
Turns out the improvement caught some new lints inside lintr too 😎 fixed now |
| @@ -10,7 +10,6 @@ | |||
| format specifiers (#472, @russHyde) | |||
| * New style SNAKE_CASE for `object_name_linter()` (#494, @AshesITR) | |||
| * RStudio source markers are cleared when there are no lints (#520, @AshesITR) | |||
| * New `assignment_spaces()` lintr. (#538, @f-ritter) | |||
There was a problem hiding this comment.
Not sure what to do about attribution here...
There was a problem hiding this comment.
I had the same hesitation... I'll mark down in the follow-up to be sure to thank them.
Closes #914 #930