prefix [linter_name] to all human-readable output formats #1363
Conversation
R/actions.R
Outdated
| "file=%s,line=%s,col=%s", x$filename, x$line_number, x$column_number | ||
| ) | ||
| cat(sprintf( | ||
| "::warning %s::%s,%s\n", |
There was a problem hiding this comment.
why not adjust the template instead?
| expect_equal(marker1$markers[[1L]]$line, lint1[[1L]]$line_number) | ||
| expect_equal(marker1$markers[[1L]]$column, lint1[[1L]]$column_number) | ||
| expect_equal(marker1$markers[[1L]]$message, lint1[[1L]]$message) | ||
| expect_equal(marker1$markers[[1L]]$message, paste0("[", lint1[[1L]]$linter, "] ", lint1[[1L]]$message)) |
There was a problem hiding this comment.
we really need some better test helpers... really hard to tell what the impact of the change is exactly.
something like expect_lint_output(x, "...", rstudio = TRUE), where we do some trim_some()-like logic on "..." to maximize readability would make it super clear to the test reader and super easy for the test writer.
(for another day...)
|
would you mind attaching a screenshot of the new output if you get a chance? i haven't had a chance to pull & install to check myself |
AshesITR
force-pushed
the
feature/1357-linter-name
branch
from
97faec4 to
49c5dca
Compare
|
I don't use RStudio, but here is the console output:
|
|
Code changes LGTM, but I've a terrible eye for design-y stuff. Let's get a third pair of eyes on it... @renkun-ken or @jameslamb care to weigh in? |
I'm not familiar enough with the internals of |
|
no need to know internals, only asking about the end-user appearance part. and good enough for me! |
fixes #1357
Didn't touch
checkstyle_output().CC @renkun-ken Not sure if this is relevant to the vscode linter support.