Improve Find All Refs results #11279
Conversation
There was a problem hiding this comment.
The change seems fine, but it feels like this deserves more explanation. From the change, it looks like we no longer run code to remove artifacts like "__o = ". Isn't that a significant regression? I'd like to see a bit more justification of why the right tradeoff is to trade one bug for another.
Excellent question, and definitely a valid concern the existing tests did nothing to validate. If we take the screenshots I posted in the original PR description though, what was actually happening was that Roslyn was returning us "Found a reference to What we now do instead is take the line of text from the Razor file itself, therefore there can't possibly be any In the non-cohosing test, this line would have said |
| // <SurveyPrompt Title="Blah" /> | ||
| // | ||
| // A FAR for the Title property will return just the word "Title" in the Text of the reference item, which does not | ||
| // help the user reason about the result. For such cases, its better to return the text from the Razor file, even |
There was a problem hiding this comment.
Where does classification happen in FAR? Can we provide the classification?
There was a problem hiding this comment.
Was just logging an issue for that :)
|
Thanks for the explanation! Another question: there don't seem to be any tests that demonstrate the new behavior -- unclassified or not. Should there be a test that validates the result in your scrreenshot above? |
|
No new tests, no, but new validation for the existing tests was added to ensure the Text of the reference item matches the line of text from the test input. |
|
eg, if I comment out the fix and run an existing cohosting test I get this failure:
|
Update integration test base lines to match the FAR behavior introduced with #11279
Fixes #11278
Fixes #4611
We lose classification, but get more context. Worthy trade off I think
Before:
After:
