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

Formatting ReScript file fails with error #2820

Closed
dcalhoun opened this issue Dec 11, 2020 · 3 comments
Closed

Formatting ReScript file fails with error #2820

dcalhoun opened this issue Dec 11, 2020 · 3 comments
Labels
A-formatting Area: Formatting & Auto-Indent bug Something isn't working

Comments

@dcalhoun
Copy link

dcalhoun commented Dec 11, 2020

When attempting to format a ReScript file, Onivim fails to format the file and throws an error.

Steps to Reproduce

  1. Download and install rescript-vscode extension vsix file.
  2. Launch Onivim 2.
  3. :enew
  4. :w test.res
  5. Cmd+Shift+P
  6. Type "format" and select "Formatting: Format Document."
  7. :enew
  8. Switch tab back to first file.
  9. Cmd+Shift+P
  10. Type "format" and select "Formatting: Format Document."

Expected Result

The ReScript file (res) is formatted. No errors are displayed.

Actual Result

The ReScript file (res) is not formatted. Errors are displayed.

The first format call produces: Invalid range specified.

The second format call produces: ParseFailedException("Expected null or:\n while decoding a list:\n element 0:\n in field \"range\":\n in field \"endColumn\":\n Expected an int, but got 1.7976931248623157e+308\n".

Screen Shot 2020-12-11 at 15 58 15

Environment

  • macOS 11.0.1
  • Onivim 2 0.5.1-nightly 0.5.1718
@bryphe bryphe added bug Something isn't working A-formatting Area: Formatting & Auto-Indent labels Dec 12, 2020
@bryphe
Copy link
Member

bryphe commented Dec 12, 2020

Thanks @dcalhoun for all the details and the screenshot. Very helpful!

I pushed up a fix for the parse exception - #2825 - we weren't handling some types of numerical values correctly in our JSON parsing for ranges.

I'm still seeing some quirks though with the ReScript formatter, even after that - possibly related to / similar issue as: #2196

bryphe added a commit that referenced this issue Dec 12, 2020
…ion host

__Issue:__ Onivim 2 wasn't correctly handling ranges from the extension host that were created with `Number.MAX_VALUE` (or, any value with any exponent...). A `ParseFailedException("Expected null or:\n while decoding a list:\n element 0:\n in field \"range\":\n in field \"endColumn\":\n Expected an int, but got 1.7976931248623157e+308\n".` was being produced.

__Defect:__ Our JSON parsing was only handling integers

__Fix:__ Use the `float` JSON decoder, which does handle exponents, and cast to an `int` - handling overflows.

This fixes the parse failure, and allows for some basic formatting - but I'm still seeing some quirks with the ReScript formatter, when updating the buffer and running the formatter again. 

Related #2820
bryphe added a commit that referenced this issue Mar 5, 2021
…its (#3233)

__Issue:__ When running format providers, like `prettier` or `rescript`, after multiple iterations, there could start to be duplicated or corrupted text.

__Defect:__ Some of the edits provided by the format provider, once they round-trip through our vim layer, weren't being applied back correctly in the extension host buffer updates (the `ModelContentChange` that we send to the extension host layer) - this would cause the buffer state to be de-sync'd - meaning that the view of the buffer text was different in the extension host than in the editor. The editor was correct, but the buffer text on the extension host side would have extra lines or extra concatenation, and when the next format was triggered, the formatting edits for the desync'd buffer would be applied, causing the issues in #2196

Currently, the way we send buffer updates is always linewise - and therefore a newline must always be included at the end. In the particular case of a formatting edit consolidating lines (deleting empty spaces), we wouldn't be adding that trailing newline - and this was the root cause of the desync.

While investigating, I found another case with undo - creating multiple lines, and then undoing them, triggers a similar bug.

__Fix:__ Simplify the way we decide to append a newline to correct these issues. Add a test case exercising the undo condition (once we have `$tryApplyEdit`, it'd be great to have that API exercised in a text synchronization case as well).

In addition, add some extra tooling in our `oni-dev-extension` to support troubleshooting these issues - when running with `oni2 -f --silent --debug-exthost` and running the `Developer: Show buffer updates` command, the extension host's view of the buffer will be shown in the console after each update.

__Next steps:__
- There's still a lot of room for improvement in the way we handle buffer updates - streamlining individual edits, and batched related updates into a single update call.

Fixes #2196 and the remainder of #2820
@dcalhoun
Copy link
Author

dcalhoun commented Mar 12, 2021

@bryphe I didn't necessarily test this again, but figured I'd close this based upon your comments/work within #3233. Feel free to reopen if I am mistaken.

Edit: to clarify, the original issue I reported continues to no longer occur after #2825.

@bryphe
Copy link
Member

bryphe commented Mar 12, 2021

Thanks @dcalhoun for closing! Indeed, I did a quick test w/ #3233 - and I wasn't able to reproduce the issue anymore.

Of course, keep me posted if you hit any other issues with formatting 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-formatting Area: Formatting & Auto-Indent bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants