r-lib / lintr

Static Code Analysis for R
https://lintr.r-lib.org
Other
1.2k stars 187 forks source link

Terminal newline test fails on non-english locale #661

Closed AshesITR closed 3 years ago

AshesITR commented 3 years ago

The terminal newline detection mechanism uses grepl with a hard-coded english warning message. On systems with a different R language, this warning will not be caught.

Similarly to expect_lint we should temporarily set LANGUAGE=en during get_source_expressions.

MichaelChirico commented 3 years ago

We'll see if it passes on Windows, which is a pain:

It is usually possible to change the language once R is running via (not Windows) Sys.setlocale("LC_MESSAGES", "new_locale"), or by setting an environment variable such as LANGUAGE

https://cran.r-project.org/doc/manuals/R-admin.html#Localization-of-messages

AshesITR commented 3 years ago

I can say for sure the Sys.setenv(LANGUAGE = "en") approach works on my non-english Windows 10. I think the mentioned problem is just LC_MESSAGES being unsupported on Windows.

MichaelChirico commented 3 years ago

Please also test for cacheing effects:

https://github.com/r-lib/testthat/pull/1258#issuecomment-737546865

AshesITR commented 3 years ago

I'm unsure how to test and / or remedy them. Do you have a suggestion?

MichaelChirico commented 3 years ago

I think we can make a gettext approach work:

LANGUAGE=zh_CN R
> gettextf("incomplete final line found on '%s'", 1, domain="R")
# [1] "读'%s'时最后一行未遂"

The only complication is that it's a templated message

AshesITR commented 3 years ago

So you mean to grepl for whatever gettextf() returns? If yes, we should do the same in all other places where we currently use the LANGUAGE workaround.

AshesITR commented 3 years ago

I've just tried in a fresh R session:

gettextf("incomplete final line found on '%s'", 1, domain="R")
#> [1] "unvollständige letzte Zeile in '1' gefunden"
Sys.setenv(LANGUAGE = "en")
gettextf("incomplete final line found on '%s'", 1, domain="R")
#> [1] "incomplete final line found on '1'"
Sys.unsetenv("LANGUAGE")
gettextf("incomplete final line found on '%s'", 1, domain="R")
#> [1] "unvollständige letzte Zeile in '1' gefunden"

So there seems to be no caching issue, at least on R 4.0.3 @ Windows 10

MichaelChirico commented 3 years ago

is that windows too?

On Fri, Dec 4, 2020, 1:32 PM AshesITR notifications@github.com wrote:

I've just tried in a fresh R session:

gettextf("incomplete final line found on '%s'", 1, domain="R")

> [1] "unvollständige letzte Zeile in '1' gefunden"

Sys.setenv(LANGUAGE = "en")

gettextf("incomplete final line found on '%s'", 1, domain="R")

> [1] "incomplete final line found on '1'"

So there seems to be no caching issue, at least on R 4.0.3

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jimhester/lintr/issues/661#issuecomment-738946431, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2BA5PTKV3GUUW55HSTSZDSTETKRANCNFSM4UNXCKMQ .

AshesITR commented 3 years ago

Yep.

MichaelChirico commented 3 years ago

OK great. Maybe one last test on an old version too?

I'm pretty apprehensive because this sort of issue has caused major headaches in the recent past:

https://github.com/Rdatatable/data.table/pull/4524

AshesITR commented 3 years ago

I don't have any old versions at hand atm. However, the issue you reference seems to be related to installed or available system languages.

On my system, I can get the error messages in french without a problem using LANGUAGE=fr, but no luck getting chinese output. Neither of cn, zh_CN, zh give a chinese message.

MichaelChirico commented 3 years ago

OK that makes more sense. presumably you'd have to have chinese language installed for the messages to appear? though maybe not if it's just doing a lookup of the env and matching it to the po file.

anyway I'm more confident we can proceed with set/unsetting the language