Add text argument to lint #503
Conversation
|
If https://github.com/jimhester/lintr/blob/42b49c1ba6d74a4910419d7ef78df804914d2b4e/R/lint.R#L30-L36 |
|
@renkun-ken is this ready for review? if so please help to merge to HEAD 😄 |
|
I update this PR to latest master.
I guess we might need to keep it that way for a while and might give a warning in the future if Now the behavior is:
If you think this behavior makes sense, then I'll update some test cases to fix the tests. |
|
I test the latest commit with languageserver using the |
Yes, please. This PR is ready for review and merge now. |
|
please remove .DS_Store |
R/lint.R
Outdated
| #' @param text Optional argument for supplying a string or lines or lines directly, | ||
| #' e.g. if the file is already in memory or linting is being done ad hoc. | ||
| #' If \code{text} is supplied but \code{filename} is missing, then \code{tempfile()} | ||
| #' will be used as the \code{filename}. |
There was a problem hiding this comment.
it looks like <text> will be used as the file name, this reads like the output would have the tempfile path instead... could you clarify what we mean here?
There was a problem hiding this comment.
It is indeed a bit ambiguous here. Here I mean that if text is provided but filename is not, then we'll write the text to a temp file (same behavior as before when filename is detected to be code), but the lint result will show as <text> which I believe should be the desired output rather than showing the tempfile in the lint result.
Looks like I should update this doc here, maybe just removing the text without filename case in the doc looks enough since writing the text to a file is just an internal implementation?
There was a problem hiding this comment.
a string or lines or lines directly looks a bit redundant?
There was a problem hiding this comment.
I think so. Removed the last sentence
|
almost there, thanks! |
tests/testthat/test-lint_file.R
Outdated
| @@ -119,6 +119,7 @@ test_that("lint() results from file or text should be consistent", { | |||
| "x<-1", | |||
| "x+1" | |||
| ) | |||
| on.exit(unlink(file)) | |||
There was a problem hiding this comment.
I didn't use on.exit() here because I want to ensure that lintr does not assume that the file exists when linting with both filename and text supplied. Therefore, I remove the file before linting so that if lint reads the file somehow it would fail as it should, which is the point of lint_from_text2.
There was a problem hiding this comment.
ah ok, please revert
|
Thanks for the review! |
|
Looks like we forgot to add a news item. I'll create a quick PR for this soon. |
Close #475
This PR adds a
text=argument tolintr::lintso that user could lint a file with suppliedtextas if the file is being linted. This is particularly useful for code editors to lively lint a file being edited but not saved while keeping the.lintrconfig (if found) effective.text=argument tolintr::lint