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

Update to use zero based indexes #4300

Merged
merged 1 commit into from
Dec 17, 2020

Conversation

david-driscoll
Copy link
Contributor

I'm moving forward with the lsp changes, but I figured this should land before that.

OmniSharp has a flag that tells the serializer to serialize with or without zero based indexes. Internally OmniSharp works purely on zero-based indexes because it makes interacting with Roslyn easier. However for backward compatibility externally we surface one-based indexes, the -z flag tells OmniSharp to return zero-based indexes instead of one based.

@JoeRobich
Copy link
Member

@NTaylorMullen @ryanbrandenburg Would this break the location mapping performed by the Razor extension?

@david-driscoll
Copy link
Contributor Author

Looking at the code it shouldn't all the middleware execution is done after everything is converted to vscode types, which are zero-based already.

@codecov
Copy link

codecov bot commented Dec 16, 2020

Codecov Report

Merging #4300 (f0abf6f) into master (942a0cb) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4300   +/-   ##
=======================================
  Coverage   85.99%   85.99%           
=======================================
  Files          60       60           
  Lines        1857     1857           
  Branches      215      215           
=======================================
  Hits         1597     1597           
  Misses        200      200           
  Partials       60       60           
Flag Coverage Δ
integration 100.00% <ø> (ø)
unit 85.99% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 942a0cb...f0abf6f. Read the comment docs.

@JoeRobich JoeRobich merged commit b04c301 into dotnet:master Dec 17, 2020
Copy link
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

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

Sorry I never ended up looking at this, I got distracted by a couple video games... LGTM though.

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.

3 participants