-
Notifications
You must be signed in to change notification settings - Fork 1
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: Add handling for sub-regions that only consist of "a" #1
base: main
Are you sure you want to change the base?
Conversation
if (!checkBlacklist(region.tokens)) { | ||
const length = region.end - region.start + 1; | ||
const replaceWith = getNumber(region).toString(); | ||
replaced = splice(replaced, region.start + offset, length, replaceWith); | ||
offset -= length - replaceWith.length; | ||
} |
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.
This is the change that actually fixed the issue.
if (end !== start) { | ||
acc.push({ | ||
start, | ||
end: end - 1, | ||
value: unfuzzyChunk, | ||
lowerCaseValue: unfuzzyChunk.toLowerCase(), | ||
type: getTokenType(unfuzzyChunk), | ||
}); | ||
} |
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.
This logic was so convoluted. This was basically going through the whole flow, just to finally check if the start index and end index are the same (meaning the string is empty), where as you can just check that at the beginning or filter them out.
Currently, there is an issue with the inline number replacement in strings consisting of more than just numbers, where the word "a" is always replaced with a "1", even if it's on it's own. This has been mentioned a few times in issues on the original project:
This behavior really doesn't make sense, and it's already handled for cases with only the word "a" (in the existing code, the standalone word "a" isn't replaced with anything). So This is meant to fix the rest of the cases where the word has no neighboring numbers to make it into an actual number.
I added some new test cases around this to ensure it works as expected.
Besides the main change, I made some more cleanup changes to make the code more readable, and organized some files.