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

Cannot save file in UTF-8 correctly #507

Merged
merged 1 commit into from
Dec 30, 2017
Merged

Conversation

snjeza
Copy link
Contributor

@snjeza snjeza commented Dec 23, 2017

Fixes redhat-developer/vscode-java#394

The issue can be reproduced on Windows.

Signed-off-by: Snjezana Peco [email protected]

@snjeza snjeza requested review from gorkem and fbricon December 23, 2017 19:40
Copy link
Contributor

@gorkem gorkem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@snjeza Can you add an explanation why we are making the change, possibly to commit message or as a comment. It is hard to see how this change fixes this issue.

@snjeza
Copy link
Contributor Author

snjeza commented Dec 30, 2017

The issue has been introduced by fixing redhat-developer/vscode-java#380
We should synchronize a compilation unit with its file system resource when calling the document/didSave command.
unit.commitWorkingCopy() synchronizes them, but, also, saves the buffer to the file system resource. The buffer is saved using the default OS encoding (cp1250 on Windows) and breaks utf8 characters on Windows.
We don't have to the save buffer to the file (the VS Code client does it), but only synchronize it. The PR does it with the following:

unit.discardWorkingCopy();
unit.becomeWorkingCopy(new NullProgressMonitor());

See also #375

@snjeza snjeza merged commit 690ad81 into eclipse-jdtls:master Dec 30, 2017
@snjeza snjeza deleted the issue-394 branch December 30, 2017 17:54
@fbricon fbricon added this to the Mid January 2018 milestone Jan 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants