Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.

Conversation

@bamarsha
Copy link
Contributor

I added two new types, Position and Range, replacing these types:

  • FParsec.Position
  • int * int
  • QsPositionInfo
  • QsPositionInfo * QsPositionInfo
  • QsRangeInfo

The new Position type is zero-based, so it has a well-defined addition operator. I replaced FParsec's getPosition parser with one that returns the new zero-based Position type directly, so the compiler never has to deal with one-based positions anymore.

This is the first step in #506. Now that these types are defined, in the next PR I'll continue updating the diagnostics code to use them, replacing the LSP position and range types as much as possible.

@bamarsha bamarsha marked this pull request as ready for review July 18, 2020 00:28
@bamarsha
Copy link
Contributor Author

bamarsha commented Jul 18, 2020

C# generation tests are currently broken for an unknown reason (they pass on my computer, but fail on the CI with an unhelpful error message). Other than that, I think it's ready to review.

@bamarsha
Copy link
Contributor Author

bamarsha commented Jul 18, 2020

All tests re-enabled and passing now.

Copy link
Contributor

@ScottCarda-MS ScottCarda-MS left a comment

Choose a reason for hiding this comment

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

Looks pretty good! Standardizing this concept across the repo really makes things easier to understand and work with. I was a bit over my head with some of the FParsec parsing stuff, but other than that everything made sense to me.

@bamarsha bamarsha merged commit 64e28b0 into feature/position-cleanup Jul 21, 2020
@bamarsha bamarsha deleted the samarsha/position-range branch July 21, 2020 17:36
bamarsha added a commit that referenced this pull request Aug 26, 2020
* Unify position and range types in parser and syntax tree (#523)

* Add new Position and Range types

* Require line and column to be non-negative

* Require start <= end

* Replace private record with DU case

* Add documentation

* Fix sln file

* Remove custom equality/comparison

* Add backwards-compatible JSON serializers

* Disable Example test target for now

* Fix RangePosition serialization

* Generate valid ranges in SymbolManagementTests

* Skip execution tests for now

* Fix null offset in QsDeclarationAttribute

* Fix position + range

* Remove range + position

* Write private constructor on one line

* Rename Range.Combine to Range.Span

* Add more documentation

* Use 1-based range positions in serialization

* Re-enable C# generation tests

* Disable execution tests again

* Revert "Disable execution tests again"

This reverts commit ec0273f.

* Update package references

* Subtract 1 from all range positions in SerializationTests

* Replace TryGetFileId with GetFileId in SerializationTests

* Rename id to ident

* Replace some LSP positions with Q# positions

* Replace LSP positions in DataStructures.cs

* Replace LSP positions in ContextBuilder.cs

* Replace LSP positions in TextProcessor.cs

* Replace LSP positions in TypeChecking.cs

* Remove type parameters from SymbolTracker and ScopeContext

* Replace LSP positions in CodeActions.cs

* Replace LSP positions in CodeCompletion.cs

* Replace LSP positions in EditorCommands.cs

* Replace LSP positions in Diagnostics.cs

* Remove unused functions

* Remove more unnecessary functions

* Remove IsSmallerThan and IsSmallerThanOrEqualTo

* Add Range.Contains

* Update docs and variable names for + and -

* Convert some LSP ranges to Q# ranges

* Rename IsValidPosition/Range with file to ContainsPosition/Range

* Remove IsValidPosition and IsValidRange

* Replace more ranges

* Avoid unnecessary conversion

* Replace GetRange() with Range

* Use properties for positions in TreeNode

* Replace HeaderEntry.GetPosition() with property

* Fix build errors after merge

* Add position in CheckDefinedTypesForCycles and ResolveAll

* Add positions in verifyResultConditionalBlocks

* Add positions in NewConjugation

* Add positions in AllPathsReturnValueOrFail

* Use lambda in CheckDefinedTypesForCycles

* Update range type

* Use original OnRangeInformation names

* Use ContainsEnd

* Specify end point is excluded in doc comment

* Preserve comments

* Rename TranslateLines to WithLineNumOffset
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants