This repository was archived by the owner on Jan 12, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 173
Clean up source code position and range handling in the compiler #531
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* 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 LSP positions with Q# positions
Replace LSP ranges with Q# ranges
# Conflicts: # src/QsCompiler/TextProcessor/QsExpressionParsing.fs # src/QsCompiler/TextProcessor/QsKeywords.fs
Merge master into feature/position-cleanup
Add offsets to diagnostic ranges in Core and SyntaxProcessor instead of CompilationManager
ScottCarda-MS
approved these changes
Aug 14, 2020
bettinaheim
reviewed
Aug 15, 2020
bettinaheim
previously requested changes
Aug 15, 2020
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All in all really nice clean-up!
The request for changes is primarily related to the API naming.
# Conflicts: # src/QsCompiler/TextProcessor/SyntaxBuilder.fs
Merge master into feature/position-cleanup
# Conflicts: # src/QsCompiler/SyntaxProcessor/StatementVerification.fs
Address review comments and merge from master for #531
bettinaheim
approved these changes
Aug 26, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PositionandRangetypes (Unify position and range types in parser and syntax tree #523)Positionwith compilerPositionin the compilation manager (Replace LSP positions with Q# positions #529)Rangewith compilerRangein the compilation manager (Replace LSP ranges with Q# ranges #530)Closes #506.