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

Fix out of bounds crash of truncated token replacement #75

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Econa77
Copy link

@Econa77 Econa77 commented Apr 2, 2021

No description provided.

@@ -48,7 +48,7 @@ extension NantesLabel {
// Walk across all the lines of truncation, replacing lines starting with our last line - the number of truncation token lines we have
// the first line we replace, we'll truncate it, after that, we 100% replace lines of the original string with truncation lines
for (index, tokenLine) in tokenLines.enumerated() {
let originalIndex = self.numberOfLines - tokenLines.count + index
let originalIndex = lines.count - tokenLines.count + index
Copy link
Author

@Econa77 Econa77 Apr 2, 2021

Choose a reason for hiding this comment

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

The settings when the app crashed were as follows.

  • self.numberOfLines: 5
  • lines.count: 4
  • tokenLines.count: 2

As a result, the originalIndex was out of range of lines and crashed at times other than the first replacement.
When there are many lines with only line breaks, the number of lines in lines may not match with numberOfLines, so the index is acquired from the number of lines in lines.

@Econa77
Copy link
Author

Econa77 commented Apr 2, 2021

The snapshot test is failing, but when I try it, it seems to fail the same before the change.

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.

1 participant