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

Off-by one bug in GetSnapshotPositionForTextPoint method #902

Closed
carlos-quintero opened this issue Jan 27, 2022 · 3 comments · Fixed by #907
Closed

Off-by one bug in GetSnapshotPositionForTextPoint method #902

carlos-quintero opened this issue Jan 27, 2022 · 3 comments · Fixed by #907
Milestone

Comments

@carlos-quintero
Copy link

In the CodeMaidShared/Helpers/TextDocumentHelper.cs file (https://github.com/codecadwallader/codemaid/blob/dev/CodeMaidShared/Helpers/TextDocumentHelper.cs), the method GetSnapshotPositionForTextPoint has an off-by-one bug, since textPoint.LineCharOffset is 1-based (https://docs.microsoft.com/en-us/dotnet/api/envdte.textpoint.linecharoffset?view=visualstudiosdk-2022#remarks):

private static int GetSnapshotPositionForTextPoint(ITextSnapshot textSnapshot, TextPoint textPoint)
{
   ThreadHelper.ThrowIfNotOnUIThread();

   var textSnapshotLine = textSnapshot.GetLineFromLineNumber(textPoint.Line - 1);
   return textSnapshotLine.Start.Position + textPoint.LineCharOffset;  // missing to substract 1
}

The reverse GetEditPointForSnapshotPosition method is fine, though, since it adds +1 to return the textPoint.LineCharOffset value.

@codecadwallader
Copy link
Owner

Carlos! It is a pleasant surprise to see you here. I want you to know that all of your examples and documentation on your site were a huge help for getting CodeMaid started. Back in the VS2005 days there was very little official support and information was hard to come by so I relied heavily on information that you put out there. I can't thank you enough for all that you've done for the community and I.

Thanks for the callout. This file underwent a lot of changes with the VS2022 changes and I appreciate the catch.

@carlos-quintero
Copy link
Author

Hi Steve

Incidentally I removed the MZ-Tools articles from the website last week, they were outdated and, belonging to the VS add-ins era, very DTE-biased :-) but I am glad that they very useful for so many years.

Well, now Microsoft references your pull request 12e226a in their documentation (https://docs.microsoft.com/en-us/visualstudio/extensibility/migration/breaking-api-list?view=vs-2022#legacy-find-api-deprecation) to show how to replace EnvDTE.Find/Replace legacy APIs. Your code has helped me a lot! Thank you very much.

@codecadwallader
Copy link
Owner

Wow, thanks for pointing that documentation out. It was actually a Microsoft employee @olegtk whom did the bulk of those API deprecation updates which was super helpful for us to become compatible with VS2022. Thanks again for your off-by-1 fix and hope you're doing well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants