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

Ensure we include the line count and end char index in remapped spans #9863

Merged
merged 2 commits into from
Jan 30, 2024

Conversation

chsienki
Copy link
Member

@chsienki chsienki commented Jan 25, 2024

Ensure we include the line count and end char index in remapped spans

Fixes #9827

@chsienki chsienki requested a review from a team as a code owner January 25, 2024 22:29
@chsienki
Copy link
Member Author

@dotnet/razor-compiler for reviews. /cc @ryzngard

@chsienki chsienki linked an issue Jan 25, 2024 that may be closed by this pull request
@@ -564,7 +564,7 @@ private static SourceSpan RemapFilePathIfNecessary(SourceSpan sourceSpan, CodeRe
// If you try and use the line pragma in the design time docs to map back to the original file it will fail,
// as the path isn't actually valid on windows. As a workaround we apply a simple heuristic to switch the
// paths back when writing out the design time paths.
sourceSpan = new SourceSpan(sourceSpan.FilePath.Replace("/", "\\"), sourceSpan.AbsoluteIndex, sourceSpan.LineIndex, sourceSpan.CharacterIndex, sourceSpan.Length);
sourceSpan = new SourceSpan(sourceSpan.FilePath.Replace("/", "\\"), sourceSpan.AbsoluteIndex, sourceSpan.LineIndex, sourceSpan.CharacterIndex, sourceSpan.Length, sourceSpan.LineCount, sourceSpan.EndCharacterIndex);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we have a test for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

We do have tests, but they were all at the start of the doc so the baseline didn't actually change. I'll add a test that covers it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated test

@chsienki
Copy link
Member Author

@jaredpar for any other feedback

@chsienki chsienki merged commit dc1040e into dotnet:features/fuse Jan 30, 2024
12 checks passed
chsienki added a commit to chsienki/razor-tooling that referenced this pull request Jan 31, 2024
…dotnet#9863)

* Ensure we include the line count and end char index in remapped spans
* Make the source span in tests have non-default values to ensure they get mapped too
chsienki added a commit that referenced this pull request Jan 31, 2024
…#9863) (#9887)

* Ensure we include the line count and end char index in remapped spans
* Make the source span in tests have non-default values to ensure they get mapped too
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FUSE] Line mappings can be generated incorrectly
4 participants