Consistent handling of Encodings #782
Conversation
|
The test file is GPL 2 licensed. I'll replace the file by a self-made one to avoid license violations of spatialprobit. |
AshesITR
force-pushed
the
feature/encoding
branch
from
1a25d40
to
937419b
Compare
handle invalid encoding in parse_exclusions() and get_source_file() suppress linting of file if invalid encoding detected also had to fix deprecated warnings caused by R 4.1.0 and testthat::expect_equal() with functions
…expects parse error instead of encoding error to be reported)
…e correct encoding.
…it failed the codecov build)
AshesITR
force-pushed
the
feature/encoding
branch
from
42539d8
to
c5d5cea
Compare
AshesITR
changed the title
| @@ -77,6 +87,36 @@ get_source_expressions <- function(filename, lines = NULL) { | |||
| # an error that does not use R_ParseErrorMsg | |||
| if (is.na(message_info$line)) { | |||
|
|
|||
| if (grepl("invalid multibyte character in parser at line", e$message, fixed = TRUE)) { | |||
There was a problem hiding this comment.
One way to increase robustness is what we've done in data.table:
Basically, keep a database (maybe populated at onAttach()) of error messages as generated by R itself.
That way we don't need to change anything if R updates its own message.
Just a thought, no need to do so in this PR.
There was a problem hiding this comment.
Nice idea. That would also allow us to remove the set_lang() construct currently in use to force english error messages.
| The file is read in using the \code{encoding} setting. | ||
| This setting found by taking the first valid result from the following locations | ||
| \enumerate{ | ||
| \item The \code{encoding} key from the usual lintr configuration settings. |
There was a problem hiding this comment.
link to ?default_settings here
There was a problem hiding this comment.
?default_settings needs a little love I think. Currently it contains not much more than "An object of class list of length 12".
Maybe that would be a good place in general to document settings?
There was a problem hiding this comment.
I picked ?default_settings kind of at random, more that "from the usual lintr configuration settings" won't be helpful if this is the first help page a user sees -- some link to help about what we mean there is needed.
If we don't have such a page yet then yes, let's use ?default_settings :)
Implements #752
fixes #541
Added TODO comments to all places that need changing to not miss anything.
Feel free to add more TODO comments if I missed some place.