Skip to content
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

uft8 source file support #328

Merged
merged 25 commits into from Nov 6, 2018

Conversation

schloerke
Copy link
Collaborator

@schloerke schloerke commented Nov 1, 2018

Fixes: #312

This stems from wanting to read utf8 files on a non utf8 locale. On windows, the locale is set to the system default which typically does not support utf8 out of the box.

utf8.R functions are adopted from the RStudio Shiny repo in an effort to use code that has already be validated to work.

The end result is that source(file, encoding="UTF-8") does not work as expected and instead we should use eval(parse(file, encoding = "UTF-8"))

Other additions

  • Use testthat::test_path when loading test files
  • Add Barret as ctb
  • Removes appveyor.yml build
  • Only source all expressions in a plumber file once - cde0d3d

shrektan and others added 13 commits Oct 12, 2018
This stems from wanting to read utf8 files on a non utf8 locale.  On windows, the locale is set to the system default which typically does not support utf8 out of the box.

sourceUTF8 functions are adopted form the rstudio/shiny repo in an effort to use code that has already be validated to work.

The ending finding is that `source(file, encoding="UTF-8")` does not work as expected and instead should use `eval(parse(file, encoding = "UTF-8"))`
@codecov-io
Copy link

@codecov-io codecov-io commented Nov 1, 2018

Codecov Report

Merging #328 into master will decrease coverage by 1.18%.
The diff coverage is 56.41%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #328      +/-   ##
==========================================
- Coverage   90.04%   88.86%   -1.19%     
==========================================
  Files          26       27       +1     
  Lines        1145     1176      +31     
==========================================
+ Hits         1031     1045      +14     
- Misses        114      131      +17
Impacted Files Coverage Δ
R/parse-block.R 96.44% <100%> (+0.01%) ⬆️
R/plumber.R 84.65% <100%> (-0.09%) ⬇️
R/utf8.R 50% <50%> (ø)
R/paths.R 75% <0%> (-1.93%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aefb2e7...b1fb950. Read the comment docs.

@schloerke schloerke requested a review from wch Nov 1, 2018
R/plumber.R Outdated Show resolved Hide resolved
@schloerke schloerke mentioned this pull request Nov 2, 2018
@schloerke schloerke requested a review from wch Nov 2, 2018
tests/testthat/test-multibytes.R Outdated Show resolved Hide resolved
@wch
Copy link
Collaborator

@wch wch commented Nov 2, 2018

We now understand why source() was being called. It shouldn't be necessary if activateBlock() is modified to do eval(expr, envir) when there's no plumber metadata associated with the expression.

@schloerke schloerke merged commit 6bfaade into rstudio:master Nov 6, 2018
2 of 4 checks passed
@schloerke schloerke deleted the shrektan-add-param-encoding branch Nov 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants