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

Correctly use UTF-8 indices when highlighting diagnostic source #683

Merged
merged 5 commits into from Aug 5, 2023

Conversation

arthurcro
Copy link
Contributor

Bug/issue #, if applicable: #678

Summary

Previously, the default diagnostic formatter would crash due to incorrect indices handling when trying to highlight a diagnostic source range.

A diagnostic's SourceRange is the range in the source file that emits the diagnostic. When highlighting a diagnostic source range, we use the column property of the upperBound and lowerBound to find indices of a source string that should be highlighted.

The column property is The number of bytes in UTF-8 encoding.

/// The number of bytes in UTF-8 encoding from the start of the line to the character at this source location.

However, this was not taken into account previously when calculating the indices.

This pull request fixes this crash by using proper UTF-8 indices.

Testing

This functionally can be tested using the steps mentioned in #678.

  1. Clone https://github.com/apple/swift.git
  2. Install a recent development snapshot of Swift
  3. Run DocC from this snapshot this way (assuming swift is the clone from step 1):
    docc preview --allow-arbitrary-catalog-directories swift/docs

Alternatively, the issue is easily reproducible by adding an emoji to a source line that would emit a diagnostic.

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

  • Added tests
  • Ran the ./bin/test script and it succeeded
  • Updated documentation if necessary

@d-ronnqvist
Copy link
Contributor

@swift-ci please test

@d-ronnqvist
Copy link
Contributor

@swift-ci please test

@d-ronnqvist
Copy link
Contributor

@swift-ci please test

@d-ronnqvist
Copy link
Contributor

@swift-ci please test

@d-ronnqvist d-ronnqvist merged commit 638779e into apple:main Aug 5, 2023
2 checks passed
@Kyle-Ye Kyle-Ye linked an issue Aug 7, 2023 that may be closed by this pull request
2 tasks
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 this pull request may close these issues.

convert and preview commands crash on certain Markdown pages
2 participants