Fix GOADB order of processing multiple unicode assignments #1615
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Tests seem to only fail on Linux which I think was mentioned somewhere else, right? |
|
The Linux failures look like they're probably specific to this PR. Haven't determined why yet. |
skef
previously approved these changes
Mar 1, 2023
|
Aha, duh. I'll fix and push. |
josh-hadley
approved these changes
Mar 1, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1273
The code was setting the single Unicode value taken from parsing the final name and then moving on to the next glyph. I swapped the order so it first checks if there are multiple Unicodes to be assigned. One other thing we could consider is the following scenario
uniABCD uniABCD uni1234,uni5678Here we will assign U+1234 and U+5678, but not U+ABCD so we could maybe warn if the name looks like a Unicode value and we don't assign it.