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: [type:nc] – ignores combining characters #3422

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

twwn
Copy link
Contributor

@twwn twwn commented Sep 16, 2024

Building on my previous PR's cleanup, adds a comparison variant to [type] which ignores when combining characters of the expected field are missing from the typed input. It still shows these characters in the "expected" line for reference.

It's useful for languages with e.g. diacritics that are required for reference (such as in dictionaries), but rarely actually learned or used in everyday writing. Among these languages: Arabic, Hebrew, Persian, Urdu.

Previously:
pre

Now:
after
[This also removes the need to have both variants of words/sentences present as separate fields, to show them redundantly, etc.]

The bool combining controls it as new final parameter of both relevant compare_answer functions. In Python, it's set to true by default.

Use on the note templates: [type:nc:field]

@twwn twwn force-pushed the typeanswer-ignore-combining branch 2 times, most recently from 0cd34f0 to 1922d25 Compare September 16, 2024 16:00
@dae
Copy link
Member

dae commented Sep 20, 2024

Thanks twwn, I'll try to review this soon. (For my own reference, previous PR: #680)

@twwn twwn mentioned this pull request Sep 20, 2024
no functional change
no functional change
No use to run all that code without input.
Either a new check or the call was too complex previously for it to trigger?
@twwn twwn force-pushed the typeanswer-ignore-combining branch from 1922d25 to 331f027 Compare September 22, 2024 12:04
bpnguyen107 and others added 13 commits September 22, 2024 16:01
* elide middle of deck names

* Update CONTRIBUTORS

* made elide mode enum

* add elide mode field

* fix enum number

* remove dataclass decorator

* Update CONTRIBUTORS

* format rust code

* Update CONTRIBUTORS

* formatting

* Update CONTRIBUTORS

* fix type hint

* Update CONTRIBUTORS
…ankitects#3394) (ankitects#3398)

* Add comment about the usage of the input field in the statistics page (ankitects#3394)

* Fix formatting issues (ankitects#3394)

* Update ts/routes/graphs/RangeBox.svelte

Co-authored-by: Mike Hardy <github@mikehardy.net>

* Update ts/routes/graphs/RangeBox.svelte

Co-authored-by: Mike Hardy <github@mikehardy.net>

---------

Co-authored-by: Damien Elmes <dae@users.noreply.github.com>
Co-authored-by: Mike Hardy <github@mikehardy.net>
* Fix occlusion rounding bug

* Fix contributors
* Fix clipboard pasting from the primary selection

* Small renaming

* Fix submodules

* Fix pylint false positive
* Update deck-config.ftl

* Update deck-config.ftl

* remove the warning
* Delay optimal FSRS params alert to ensure progress updates are reported

* Ensure progress updates arrive synchronously
* Add "Show in folder" option to images in editor

Credits: @abdnh's Reveal in File Manager add-on (https://github.com/abdnh/anki-misc/tree/master/reveal_in_file_manager)

* Refactor
Adds a comparison variant to [type] which ignores when combining characters of the expected field are missing from the typed input. It still shows these characters in the 'expected' line for reference.

It's useful for languages with e.g. diacritics that are required for reference (such as in dictionaries), but rarely actually learned or used in everyday writing. Among these languages: Arabic, Hebrew, Persian, Urdu.

The bool 'combining' controls it as new final parameter of both relevant compare_answer functions. On the Python side, it's set to true by default.

Use on the note templates: [type:nc:field] (only the front needs to include :nc)

This also removes the need to have both variants of words/sentences present as separate fields, to show them redundantly, etc.
Requires adjusting two testcases, but both render exactly the same in Anki itself.

On NFC vs. NKFD: https://stackoverflow.com/a/77432079
Should get rid of most relocations, but at the expense of over-allocating.

On Vec's (String's) behavior: https://stackoverflow.com/a/72787776
@twwn twwn force-pushed the typeanswer-ignore-combining branch from 331f027 to 817e719 Compare September 22, 2024 14:03
@twwn
Copy link
Contributor Author

twwn commented Sep 22, 2024

The minor commits are suggestions, each with a link to relevant Stack Overflow threads in their messages.

  • "simplify by using nfkd throughout" does so by removing the use of NFC in the original case as well. Though that required adjusting two testcases, this only reflects internal processing: I compared both before/after and the rendered end results are the same.
  • I'm partial to leaving in normalize_provided as member function as it means normalize (as normalize_expected) won't have to carry around the bool.

I'll close this PR & reopen a new one after review, the incremental rebases are rather tedious.

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.

9 participants