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

Prevent parseUTF8 from overwriting srcfile attribute #930

Merged
merged 3 commits into from Nov 20, 2023

Conversation

meztez
Copy link
Collaborator

@meztez meztez commented Nov 18, 2023

Fix #916, related to parseUTF8 return value attribute srcfile on Windows.

PR task list:

  • Update NEWS
  • Add tests
  • Update documentation with devtools::document()

@ArcadeAntics
Copy link

ArcadeAntics commented Nov 18, 2023

This fixes the issue of the source file pointing to the correct file, but on other operating systems you've basically made a srcfilecopy just for parse to overwrite it with the exact same input. You need to directly supply keep.source = FALSE always, on every OS, for all possible encodings. You're losing all the work done by readUTF8 by throwing out those lines.

@ArcadeAntics
Copy link

Yes, perfect! Thank you so much!

@meztez
Copy link
Collaborator Author

meztez commented Nov 18, 2023

This fixes the issue of the source file pointing to the correct file, but you've basically made a srcfilecopy just for parse to overwrite it with the exact same input. You need to directly supply keep.source = FALSE always.

I saw that, I thought maybe there was a reason for it? I will let @schloerke comment on this behaviour since it seems the line came from a commit he authored. I've taken your comment into account for this PR.

Shiny has keep.source set to FALSE with similar instructions in sourceUTF8.
https://github.com/rstudio/shiny/blob/b1297395a9365c97972f243ee492232d70af281f/R/utils.R#L1355

@schloerke
Copy link
Collaborator

Yes, this puts us back to parity with Shiny's implementation. Don't know why I deviated in the first place.

Thank you!

@schloerke schloerke changed the title Prevent parseUTF8 to overwrite srcfile attribute of return value on Windows Prevent parseUTF8 from overwriting srcfile attribute Nov 20, 2023
@schloerke schloerke merged commit 25117ed into rstudio:main Nov 20, 2023
12 checks passed
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.

in parseUTF8() on Windows with enc "unknown", src is ignored / / overwritten
3 participants