-
Notifications
You must be signed in to change notification settings - Fork 417
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
Don't remap line mappings in Razor files #2460
Don't remap line mappings in Razor files #2460
Conversation
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.
I am not convinced this is the best thing to do, and it is demonstrated by the test you modified. Line mappings can be injected at any point in the code, for any arbitrary reason and I think the language service should respect that, instead of making a deliberate decision to ignore them.
Now, since we already exclude it from Cake, how about we exclude that from Razor files too, instead of making a global change to ignore those mappings? That would then solve your use case without affecting the 'normal' C# code.
Thanks for the suggestion, Filip - that makes sense. I took your feedback and excluded Razor files. I also added tests specifically for the Razor scenario. Could you take another look? |
Also, it doesn't have to be addressed in this PR, but it seems that VS and VS Code have different behaviors for line mappings in *.cs files. VS doesn't respect line mappings while VS Code does - for example, when running FindAllRefs in VS Code with the following: public class Foo
{
public void bar() { }
}
public class FooConsumer
{
public FooConsumer()
{
#line 100
new Foo().bar();
#line default
}
} Not sure if we want to make these consistent eventually, just food for thought. |
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.
thanks!
Problem: O# sets the start/end characters of a location to 0 if the location is within a line mapping directive. This is problematic in Razor, where we're attempting to implement document highlighting for the first time and need exact start/end characters.
Solution: Use the original line and character locations (edit: only in Razor files). This also means we need to return the non-mapped file path.
I confirmed features that go through this call path (document highlighting, find all references, go-to-impl, etc.) still work correctly with this change in both C# and Razor files. Most features still work since many do their own re-mapping behind the scenes. As a result of this change, document highlighting in Razor now works too. 🙂