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

Git diff ignores language configured encoding #48038

Closed
vlopes11 opened this issue Apr 17, 2018 · 7 comments
Closed

Git diff ignores language configured encoding #48038

vlopes11 opened this issue Apr 17, 2018 · 7 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug git GIT issues good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities
Milestone

Comments

@vlopes11
Copy link

vlopes11 commented Apr 17, 2018

  • VSCode Version: 1.22.2
  • OS Version: Windows 10 x64 version 1709

Version 1.22.2
Commit 3aeede7
Date 2018-04-12T16:38:45.278Z
Shell 1.7.12
Renderer 58.0.3029.110
Node 7.9.0
Architecture x64

Steps to Reproduce:

  1. Setup user settings according to this
    "[json]": {
        "files.encoding": "windows1252"
    },
    "files.encoding": "utf8",
  1. Initialize a git repo
  2. Create a test.json file with the following content (win1252 encoding) and commit
{
	"some-test": "Some content with win1252 characters",
	"content": "ãçÇ~'aàá",
	"test": "regular chars"
}
  1. Change only line 4 for something (like this)
{
	"some-test": "Some content with win1252 characters",
	"content": "ãçÇ~'aàá",
	"test": "regular chars test"
}
  1. Check the git diff. Will report changes on lines 3 and 4 because will convert to utf8 before diff, ignoring the language encoding configuration (win1252)
  2. Perform a git diff to see the difference and how it should work properly (only line 4 reported as changed):
$ git diff test.json
diff --git a/test.json b/test.json
index 638f49c..b19527f 100644
--- a/test.json
+++ b/test.json
@@ -1,5 +1,5 @@
 {
        "some-test": "Some content with win1252 characters",
        "content": "<E3><E7><C7>~'a<E0><E1>",
-       "test": "regular chars"
+       "test": "regular chars test"^M
 }
  1. Issue won't happen if my user settings are like this:
    "[json]": {
        "files.encoding": "windows1252"
    },
    "files.encoding": "windows1252",

Does this issue occur when all extensions are disabled?: Yes

@vscodebot vscodebot bot added the git GIT issues label Apr 17, 2018
@joaomoreno joaomoreno added the bug Issue identified by VS Code Team member as probable bug label Apr 18, 2018
@joaomoreno joaomoreno added this to the Backlog milestone Apr 18, 2018
@joaomoreno joaomoreno added good first issue Issues identified as good for first-time contributors help translation Issues related to translation - translation help wanted help wanted Issues identified as good community contribution opportunities and removed help translation Issues related to translation - translation help wanted labels Apr 18, 2018
@vlopes11
Copy link
Author

I did some quick research, seems the diff command creates a URI of scheme git and other of type file (for current one)

Editor instance is considering user setting language encoding only for file schemes; therefore git scheme is being rendered with default configuration.

Later I'll try to PR on this.

@mkavidas
Copy link
Contributor

@vlopes11 Are you currently working on this? I am looking to get started with the project and would like to maybe tackle this as my first issue?

@vlopes11
Copy link
Author

@mkavidas That would be amazing! I'm quite short on available time so you could fully dive into this.

As a tip, seems like the resourceEditorInput class don't have getEncoding and setEncoding methods implemented.

diffEditorInput will instantiate this class for original EditorInput and fileEditorInput for modified one in this bug example

If these methods exists, seems like standard Editor creation will use them to process the encoding.

@mkavidas
Copy link
Contributor

mkavidas commented Apr 28, 2018

@vlopes11 thanks for the help. I've been trying to just figure out how the code works/ how to fix it.

I first tried to make resourceEditorInput implement IEncodingSupport then I added the necessary getEncoding and setEncoding methods. It seems that despite the methods being there they are never called.

Did you mean to modify the constructor to add a "preferredEncoding" argument?

It also thought this might be the culprit: https://github.com/Microsoft/vscode/blob/8578f0241e1a64fb5af8ee1b030e42579ef9cd6f/src/vs/workbench/services/editor/common/editorService.ts#L329

And while it is true that resourceInput.encoding is undefined, even if I hard code it to be 'windows1252' it has no effect.

I feel like I might be missing something so any advice you can give me would be greatly appreciated!

I'm mostly curious on how the resourceEditorInput grabs the default files.encoding (while ignoring the language specific). I think if I can find where that happens I could add some logic to grab the correct encoding.

@ArcticLampyrid
Copy link

Is there any progress?
I am also troubled by this problem.

@MikolajTwarog
Copy link

I would like to take care of this issue, but I'm new to this project, so any help would be appreciated.

@joaomoreno
Copy link
Member

Same underlying issue as #36219, now fixed thanks to #55110

@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug git GIT issues good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities
Projects
None yet
Development

No branches or pull requests

5 participants