-
Notifications
You must be signed in to change notification settings - Fork 185
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
Turn off line length linter within string literals (optionally?) #856
Comments
Not sure if we should, but we have a similar problem with long machine-readable comments, so it would be nice to also control linting of comment lines. What should qualify a line for exclusion in both cases? |
by "bodies" i meant to exclude the first and last line, e.g.
So it seems like we could add two options to the linter: (1) exclude_comments = FALSE (by default) |
Giving this a bit more thought, one use case with the string line length issue is with very long paths, where adding newlines to the string is not an option. To clarify what I mean:
IINM this behavior could be achieved by treating every string literal as length 0 |
We do have loads of long file names, but usually it's because of the leading directories, and that can be solved using OTOH, we also have a lot of SQL table names / nested column structures accessed with |
Related: for me the place where the line_length always bites me is with urls. I often find myself doing something like: combined_sd <- function(mean, sd, n) {
# based on
# https://math.stackexchange.com/questions/2971315/how-do-i-combine-standard-deviations-of-two-groups # nolint
...
} |
NB for Stackexchange specifically, you can shorten the URL drastically: https://math.stackexchange.com/questions/2971315/how-do-i-combine-standard-deviations-of-two-groups Is equivalent to https://math.stackexchange.com/q/2971315 The no spaces heuristic breaks down for long paths that contain embedded spaces. |
Hmm, I might lint in the first case, requiring the string literal to be "alone" on the line to be exempted: read_csv(
"very_long_path.csv"
) "alone" because the latter case, where the string is the target of an read_csv(
file =
"very_long_path.csv"
) Note to self that we'll also have to exclude the |
@d-sci ICYMI you can now (in dev & soon on CRAN) switch to # nolint next: line_length_linter.
# https://... |
We've got a very common scenario where string literals are used to contain big SQL queries.
Crucially, the style guide for SQL & R differ in that the former's max line width is 100, the latter 80.
Moreover, to skip only one such line we've got to somewhat awkwardly inject the nolint tag inside the SQL:
It seems to me the best solution here is to simply ignore string literal "bodies", perhaps with an option.
Happy to implement this if others agree.
The text was updated successfully, but these errors were encountered: