New paste_sep_linter #997
New paste_sep_linter #997
Conversation
R/paste_sep_linter.R
Outdated
| sep_cond <- file.path( | ||
| "SYMBOL_SUB[text() = 'sep']", | ||
| # NB: length-2 means '' or "" (*not* 'ab' or 'cd' which have length 4) | ||
| "following-sibling::expr[1][STR_CONST[string-length(text()) = 2]]" |
There was a problem hiding this comment.
Edge case: What about r"()"?
Probably easier to understand with explicit comparisons.
There was a problem hiding this comment.
yea... this is gonna come up a few times. we're still on R 3.6.3 so haven't had to lint for any raw strings stuff yet.
in the case of this linter, I think it's fine as is... paste(sep = R'()') seems unlikely.
this linter has mainly been useful for tidying up very old code written before paste0 existed, FWIW
PS we may have to do a thing where we merge first and tidy up later, for any case where there's something unique to do for R>4.0. it'll be tough to do such changes in my work environment.
There was a problem hiding this comment.
I can chip in with 4.x stuff.
It should also be safe and future proof to instead check identical(eval(xml_text(sep_str_const)), ""). WDYT?
There was a problem hiding this comment.
right, but i always hate pulling stuff out of the XPath if we can avoid it 😃
Part of #962