Skip to content

Fix cloze numbering in Instant Note when text already contains clozes#20360

Closed
Alok-Silswal wants to merge 2 commits intoankidroid:mainfrom
Alok-Silswal:fix-instant-cloze-sync
Closed

Fix cloze numbering in Instant Note when text already contains clozes#20360
Alok-Silswal wants to merge 2 commits intoankidroid:mainfrom
Alok-Silswal:fix-instant-cloze-sync

Conversation

@Alok-Silswal
Copy link

@Alok-Silswal Alok-Silswal commented Feb 24, 2026

Purpose / Description

Instant Note was not handling cloze numbering properly when the shared text already had clozes like {{c1::}}, {{c2::}}, etc.

This PR fixes it.

Fixes

Approach

  • The fix ensures that the cloze counter is derived from the actual text instead of assuming it always starts from 1.

  • Mode switching was simplified so that changing modes does not change the counter by itself.

Flow Summary:

setClozeFieldText(text)
        ↓
syncClozeState(text)
        ↓
Extract existing cloze indices
        ↓
Update intClozeList
        ↓
Set counter = max(existing) + 1

During Selection:

If INCREMENT ([...]+):
   - Use current counter
   - Then increment counter

If NO_INCREMENT ([...]):
   - Use highest existing cloze index
   - Do not change counter 

How Has This Been Tested?

  • Verified that existing unit tests pass (InstantEditorViewModelTest.kt).

  • Tested on my phone (Android 14, Xiaomi / HyperOS) using a debug build.

Used the following two texts for testing:

This is the {{c1::first cloze}}. This should be the second cloze.
This is the {{c1::first cloze}}. This should be the {{c2::second cloze}}. Similarly, this one as {{c3::third cloze}}.

Two screen recording attached as evidence.

Checklist

Please, go through these checks before submitting the PR.

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner
Cloze_fix_evidence_1.mp4
Cloze_fix_evidence_2.mp4

Copy link
Contributor

@criticalAY criticalAY left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The seems LLM generated, comments are still there

Comment on lines 183 to 185
/** Sync cloze counter with actual text.
When shared text already contains {{cX::}}, we must detect existing indices and start from max + 1.
Also update intClozeList so undo/reset logic works correctly. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc can be improved

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc can be improved

" Make cloze counter and intClozeList consistent with actual text. "

How about this one?

Comment on lines 193 to 204
val existingNumbers =
clozePattern
.findAll(text)
.mapNotNull { it.groups[2]?.value?.toIntOrNull() }
.toList()

// Sync the list of indices
intClozeList.clear()
intClozeList.addAll(existingNumbers)

// Set the next cloze number to max + 1
_currentClozeNumber.value = (existingNumbers.maxOrNull() ?: 0) + 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do without intermediate list allocation

Copy link
Author

@Alok-Silswal Alok-Silswal Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood! We will allocate directly then.


intClozeList.clear()

var max = 0
clozePattern.findAll(text).forEach {
    val value = it.groups[2]?.value?.toIntOrNull()
    if (value != null) {
        intClozeList.add(value)
        if (value > max) max = value
    }
}

_currentClozeNumber.value = max + 1

Is this approach correct?

@Alok-Silswal
Copy link
Author

comments are still there

I had over-explained the details. Simplified it now.

Copy link
Author

@Alok-Silswal Alok-Silswal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improved the doc and removed intermediate list allocation.

@criticalAY
Copy link
Contributor

I had over-explained the details. Simplified it now.

Can I confirm you've read our AI policy: https://github.com/ankidroid/Anki-Android/blob/main/AI_POLICY.md? The code you provided was AI-generated

@criticalAY criticalAY added the Needs Author Reply Waiting for a reply from the original author label Feb 26, 2026
@Alok-Silswal
Copy link
Author

Can I confirm you've read our AI policy: https://github.com/ankidroid/Anki-Android/blob/main/AI_POLICY.md? The code you provided was AI-generated

I have read it now.

Code logic was mine (POV: I do DSA regularly), but I used AI for implementation and for polishing PR description also in the first one(whose words were also mine).

Now I understand that up till 3 PR we cannot use AI in any form. This makes this PR ineligible.

Shall I rework this PR or close it to submit a new one?

@Alok-Silswal
Copy link
Author

I checked other PRs with same issue and seems like this will not work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Author Reply Waiting for a reply from the original author Needs Review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adding additional clozes doesn't work properly for Instant Cards in AnkiDroid

2 participants