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

Key Binding erases file contents #3

Closed
gilbertoalbino opened this issue Aug 25, 2016 · 4 comments
Closed

Key Binding erases file contents #3

gilbertoalbino opened this issue Aug 25, 2016 · 4 comments
Assignees
Labels

Comments

@gilbertoalbino
Copy link

I have installed the extension and it happens that when I use the KeyBinding, it erases all the file contents.

@vysker
Copy link
Owner

vysker commented Aug 25, 2016

Well that's not supposed to happen.

Could you provide any details? Such as the OS you're using, the keybinding, your settings, the context of the problem. Also, if you could turn on the phpformatter.logging, try formatting again, and then posting the console log, we might be able to shed a little light on the problem at hand.

If you could post the results here, then we might be able to figure this out together.

Thanks for bringing this to my attention.

@vysker vysker self-assigned this Aug 25, 2016
@vysker vysker added the bug label Aug 25, 2016
@gilbertoalbino
Copy link
Author

I running Visual Code 1.4 on Windows 10 64, with these user settings:

    "phpformatter.composer": true,
    "phpformatter.onSave": true,
    "phpformatter.level": "psr2",
    "phpformatter.logging": true

And this keybinding:

{
        "key": "ctrl+shift+l",
        "command": "phpformatter.fix",
        "when": "editorFocus"
    }

No output to console, by the way!

@Seidel-Michael
Copy link

Exact same Problem here. In Addition when I activate the onSave Feature nothing happens.

@vysker
Copy link
Owner

vysker commented Sep 8, 2016

This issue gave me huge headache, but I finally managed to fix it.

First, I tried checking whether this had anything to do with reading the file using NodeJS's fs library. According to the documentation, fs.readFileSync reads the entire file.

So, then I immediately assumed this had something to do with character encoding. After having spent several hours on that, I came to the conclusion that it could not be character encoding. I had completely isolated the issue from that.

So then I started fiddling around with reading files. Because I could positively confirm that the contents of the *.tmp files were properly written, the issue had to be with reading them. That's when I tried reading the file without using the file descriptor. Surprisingly, the entire file was read this time. Obviously, the file descriptor stored the wrong cursor position, which was used by fs.readFileSync.

The solution was to drop the fs.readFileSync method and use a read stream instead. This accepts an option to set the cursor position manually.

And that was that! Sorry for the long wait.

Also, this writing and reading from files currently assumes UTF-8. So that is not very reliable. Unfortunately, there is no way to check the character encoding of a file in Visual Studio Code programmatically, yet. I added a feature request to the Visual Studio Code GitHub just for that.

In the near future, I will have to implement a workaround for character encoding detection using a third-party package. For now, it will all be UTF-8.

@vysker vysker closed this as completed Sep 8, 2016
vysker added a commit that referenced this issue Sep 8, 2016
This resolves #3, where fixing a file would erase the contents. It was the result of the fs.readFileSync method which handled the file descriptor cursor position poorly. Using a read stream, we can set the file cursor position to the first line when reading from the temp file.

Be aware, however, that this code assumes utf-8 character encoding throughout, which could cause a lot of issues. Hence we are waiting for a feature to be implemented with which extensions can get the a file's character encoding. Feature request here: microsoft/vscode#11690.

Also added some more debug logging, as well as a convenience method that wraps a string in double quotes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants