-
Notifications
You must be signed in to change notification settings - Fork 4
fix(education): strip middot/bullet before in/of connective (#839) #841
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -533,9 +533,14 @@ function parseDegreeAndField(line: string): { | |
| } | ||
| const fieldRaw = line | ||
| .slice(fieldStart) | ||
| // Drop a leading "in "/"of " connective or a "-"/"—"/":"/"," separator. | ||
| // Drop a leading "in "/"of " connective or a "-"/"–"/"—"/","/":"/"·"/"•" | ||
| // separator. The middot/bullet live here, not only in `cleanField`, because | ||
| // this strip runs FIRST — `cleanField`'s edge strip accepted them already but | ||
| // runs after the connective strips, too late to rescue a "· in <field>" | ||
| // header (#839). The two classes still differ (`cleanField` also takes ";"); | ||
| // unifying the separator vocabulary repo-wide is #653. | ||
| .replace(/^\s*(?:in|of)\s+/i, "") | ||
| .replace(/^\s*[-–—,:]\s*/, "") | ||
| .replace(/^\s*[-–—,:·•]\s*/, "") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Secondary — the two classes still disagree, one glyph over. Not blocking: |
||
| .replace(/^\s*(?:in|of)\s+/i, ""); | ||
| return { degree, field: cleanField(fieldRaw) }; | ||
| } | ||
|
|
||
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.
Nit — this test and the interior-middot one below it pass identically before and after the change. I confirmed by reverting
education.tstoorigin/mainand running just this describe block: the two middot/bullet cases fail, these two pass. That is correct — AC 3 and AC 4 of #839 asked for them precisely so the fix is pinned as a widening rather than a rewrite — but a later reader scanning a#839describe block will reasonably read all four as evidence the fix works. One line of comment saying these two are regression guards ("unchanged by #839; here so a future narrowing of the class is caught") would carry that.Second, smaller:
expect(hyphen[0].field)andexpect(emDash[0].field)index into the array without anexpect(...).toHaveLength(1)first, unlike the two tests above. If a future change split this header into two entries,[0]would still satisfy the assertion and the test would stay green on a real regression.