refactor(libanki): convert Consts to enums #17431

Arthur-Milchior · 2024-11-12T22:51:15Z

This PR should be merged after #17430. It contains #17430 and removing #17430 would create a ton of merge conflict.

It transforms IntDef of Consts.kt into enum

Blocked by dependency Currently blocked by some other dependent / related change NF: Use com.ichi2.anki.Ease as much as possible #17430

david-allison · 2024-12-06T00:40:57Z

@Arthur-Milchior Unblocked

david-allison

Incomplete review, I'm not going to have time to finish it

david-allison · 2024-12-07T18:12:43Z

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt

@@ -246,12 +246,12 @@ open class AnkiDroidJsAPI(private val activity: AbstractFlashcardViewer) {
            "cardMod" -> convertToByteArray(apiContract, currentCard.mod)
            "cardId" -> convertToByteArray(apiContract, currentCard.id)
            "cardNid" -> convertToByteArray(apiContract, currentCard.nid)
-            "cardType" -> convertToByteArray(apiContract, currentCard.type)
+            "cardType" -> convertToByteArray(apiContract, currentCard.type.ordinal)


! Should be a constant, not dependent on the enum order

AnkiDroid/src/main/java/com/ichi2/libanki/sched/Scheduler.kt

AnkiDroid/src/main/java/com/ichi2/libanki/Consts.kt

AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt

AnkiDroid/src/main/java/com/ichi2/anki/dialogs/customstudy/CustomStudyDialog.kt

david-allison

Doesn't compile

Last round of feedback. Let's get this in

AnkiDroid/src/main/java/com/ichi2/libanki/Consts.kt

AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt

AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt

david-allison · 2024-12-12T06:48:19Z

AnkiDroid/src/main/java/com/ichi2/libanki/Consts.kt

+        fun reviewable() =
+            when (this) {
+                MANUALLY_BURIED, SIBLING_BURIED, SUSPENDED -> false
+                NEW, LRN, REV, DAY_LEARN_RELEARN, PREVIEW -> true


This doesn't make sense to me as a name. Preview is reviewable?

Would "viewable" be better?
I'd be fine with a NOTREACHED() for the "preview" case because we should never reach it.

AnkiDroid/src/main/java/com/ichi2/libanki/Consts.kt

AnkiDroid/src/main/java/com/ichi2/libanki/NotetypeJson.kt

AnkiDroid/src/test/java/com/ichi2/libanki/ModelTest.kt

david-allison

Cheers!

AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt

Arthur-Milchior · 2025-01-01T18:20:31Z

I hope I understood correctly what you asked me to do with the unknown case.
Now if the database contains data that we don't know how to interpret we'll have a Unknown queue and card type.
I was effrayed it would break ankidroid in unexpected way. But actually, as we don't do anything except display value, I guess if anything break, it'll be backend job to handle it.

david-allison · 2025-01-02T10:22:22Z

Needs ./gradlew --rerun-tasks ktLintFormat, maybe over a few commits

Would also question why the pre-commit hook isn't firing. This may want to be rebased on main if it hasn't already

This improves typing. It's also clearer what will be the behavior of the app if we encounter code with a new kind of card type.

This improve the typing.

Improve typing

david-allison · 2025-01-03T19:19:52Z

AnkiDroid/src/main/java/com/ichi2/libanki/Consts.kt

+sealed class CardType(
+    val code: Int,
+) {
+    object New : CardType(0)


prefer data object

AnkiDroid/src/main/java/com/ichi2/libanki/Consts.kt

lukstbit · 2025-01-04T11:25:54Z

Merging this now so it doesn't take any more conflicts.

Arthur-Milchior force-pushed the CONSTS branch from 5a067d4 to 053f251 Compare November 12, 2024 23:17

david-allison added the Blocked by dependency Currently blocked by some other dependent / related change label Nov 13, 2024

github-actions bot added the Has Conflicts label Dec 4, 2024

david-allison added Needs Author Reply Waiting for a reply from the original author and removed Blocked by dependency Currently blocked by some other dependent / related change Has Conflicts labels Dec 6, 2024

github-actions bot added the Has Conflicts label Dec 6, 2024

Arthur-Milchior force-pushed the CONSTS branch from 053f251 to 84e33c4 Compare December 7, 2024 00:03

Arthur-Milchior added Needs Review and removed Needs Author Reply Waiting for a reply from the original author Has Conflicts labels Dec 7, 2024

david-allison requested changes Dec 8, 2024

View reviewed changes

david-allison added the Needs Author Reply Waiting for a reply from the original author label Dec 8, 2024

github-actions bot added the Has Conflicts label Dec 9, 2024

Arthur-Milchior force-pushed the CONSTS branch from 84e33c4 to e551396 Compare December 12, 2024 00:00

david-allison requested changes Dec 12, 2024

View reviewed changes

david-allison removed the Has Conflicts label Dec 12, 2024

Arthur-Milchior force-pushed the CONSTS branch 3 times, most recently from 194ac14 to 74c7155 Compare December 12, 2024 21:14

Arthur-Milchior removed the Needs Author Reply Waiting for a reply from the original author label Dec 12, 2024

Arthur-Milchior force-pushed the CONSTS branch 2 times, most recently from 1148af6 to fdc3513 Compare December 12, 2024 21:34

david-allison added the Needs Author Reply Waiting for a reply from the original author label Dec 13, 2024

github-actions bot added the Has Conflicts label Dec 13, 2024

Arthur-Milchior force-pushed the CONSTS branch 2 times, most recently from e9f22c1 to 5cd6e10 Compare December 14, 2024 02:12

Arthur-Milchior removed Needs Author Reply Waiting for a reply from the original author Has Conflicts labels Dec 14, 2024

github-actions bot added the Has Conflicts label Dec 17, 2024

Arthur-Milchior force-pushed the CONSTS branch from d0799fc to 232c806 Compare December 29, 2024 13:17

david-allison approved these changes Dec 29, 2024

View reviewed changes

AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt Outdated Show resolved Hide resolved

david-allison added Needs Second Approval Has one approval, one more approval to merge and removed Needs Author Reply Waiting for a reply from the original author Needs Review Has Conflicts labels Dec 29, 2024

david-allison changed the title ~~Consts~~ refactor(libanki): convert Consts to enums Dec 29, 2024

github-actions bot added the Has Conflicts label Dec 30, 2024

Arthur-Milchior force-pushed the CONSTS branch from 232c806 to 4c2933a Compare January 1, 2025 18:19

Arthur-Milchior removed the Has Conflicts label Jan 1, 2025

github-actions bot added the Has Conflicts label Jan 2, 2025

Arthur-Milchior added 3 commits January 3, 2025 20:07

NF: CardType is a enum class

989f259

This improves typing. It's also clearer what will be the behavior of the app if we encounter code with a new kind of card type.

NF: QueueType becomes an enum

64ae878

This improve the typing.

NF: ModelType is an enum

07d0068

Improve typing

Arthur-Milchior force-pushed the CONSTS branch from 4c2933a to 07d0068 Compare January 3, 2025 19:16

david-allison approved these changes Jan 3, 2025

View reviewed changes

david-allison removed the Has Conflicts label Jan 3, 2025

lukstbit approved these changes Jan 4, 2025

View reviewed changes

lukstbit added Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) and removed Needs Second Approval Has one approval, one more approval to merge labels Jan 4, 2025

lukstbit added this pull request to the merge queue Jan 4, 2025

Merged via the queue into ankidroid:main with commit a01983b Jan 4, 2025
9 checks passed

github-actions bot added this to the 2.21 release milestone Jan 4, 2025

github-actions bot removed the Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) label Jan 4, 2025

Arthur-Milchior deleted the CONSTS branch January 5, 2025 19:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(libanki): convert Consts to enums #17431

refactor(libanki): convert Consts to enums #17431

Arthur-Milchior commented Nov 12, 2024 •

edited by david-allison

Loading

david-allison commented Dec 6, 2024

david-allison left a comment

david-allison Dec 7, 2024

david-allison left a comment •

edited

Loading

david-allison Dec 12, 2024

Arthur-Milchior Dec 12, 2024

david-allison left a comment

Arthur-Milchior commented Jan 1, 2025

david-allison commented Jan 2, 2025

david-allison Jan 3, 2025

lukstbit commented Jan 4, 2025

refactor(libanki): convert Consts to enums #17431

refactor(libanki): convert Consts to enums #17431

Conversation

Arthur-Milchior commented Nov 12, 2024 • edited by david-allison Loading

david-allison commented Dec 6, 2024

david-allison left a comment

Choose a reason for hiding this comment

david-allison Dec 7, 2024

Choose a reason for hiding this comment

david-allison left a comment • edited Loading

Choose a reason for hiding this comment

david-allison Dec 12, 2024

Choose a reason for hiding this comment

Arthur-Milchior Dec 12, 2024

Choose a reason for hiding this comment

david-allison left a comment

Choose a reason for hiding this comment

Arthur-Milchior commented Jan 1, 2025

david-allison commented Jan 2, 2025

david-allison Jan 3, 2025

Choose a reason for hiding this comment

lukstbit commented Jan 4, 2025

Arthur-Milchior commented Nov 12, 2024 •

edited by david-allison

Loading

david-allison left a comment •

edited

Loading