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

typeanswer: NFC fix & cleanup #3482

Merged
merged 4 commits into from
Oct 11, 2024
Merged

typeanswer: NFC fix & cleanup #3482

merged 4 commits into from
Oct 11, 2024

Conversation

twwn
Copy link
Contributor

@twwn twwn commented Oct 6, 2024

I'm sorry if this is getting annoying as my last PR was already the "final" cleanup, but this should really be it…

No functional changes:

  • DiffNonCombining's new() used String where plain Vec is appropriate
  • get rid of normalize_typed for DiffTrait again by pulling code into DiffNonCombining's new()
  • DiffNonCombining testcase
  • Typos

Functional change

  • Revert to NFC throughout typeanswer.rs for now. If it breaks the non-combining comparison for other languages, we can still re-revert only for it to NFKD later. (This time I'd skip normalize_to_nfkd/text.rs and just go for nfkd() directly, it returning an iterator also allows more concise code.)

(Curiously my local nightly rustfmt now wants to reformat other testcases in a way that breaks the repo check.)

* DiffNonCombining's new() used String where plain Vec is appropriate
* get rid of normalize_typed for DiffTrait again by pulling code into DiffNonCombining's new()
* two DiffNonCombining testcases
@twwn twwn changed the title typeanswer: cleanup typeanswer: NFC fix & cleanup Oct 8, 2024
@dae
Copy link
Member

dae commented Oct 11, 2024

I'm sorry if this is getting annoying as my last PR was already the "final" cleanup, but this should really be it…

These things tend to happen :-) And I was the one that brought up the NFC point. Appreciate you addressing issues and tidying things up.

Anki pins a specific nightly version so that the formatting is consistent - formatting is invoked with cargo/format as the working dir.

@dae dae merged commit 8af63f8 into ankitects:main Oct 11, 2024
1 check passed
@twwn twwn deleted the typeanswer-cleanup branch October 11, 2024 12:06
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.

2 participants