Skip to content

Add support for canAddNotesWithDetail with fallback to canAddNotes if not supported#2220

Merged
Kuuuube merged 2 commits intoyomidevs:masterfrom
RGExpert:refactor/duplication-logic
Oct 28, 2025
Merged

Add support for canAddNotesWithDetail with fallback to canAddNotes if not supported#2220
Kuuuube merged 2 commits intoyomidevs:masterfrom
RGExpert:refactor/duplication-logic

Conversation

@RGExpert
Copy link
Copy Markdown

If canAddNotesWithDetail is implemented in anki-connect we can save up another api call, if not fallback to using two.

canAddNotesWithDetail seems to have been used in original logic as implemented in #693.
It was reverted to using two canAddNotes #827 due to bugs some users were having due to an old anki-connect.

But I think a fallback should have been provided instead (comment in code actually mentions "if older version" but not checks are being done).

#827 also is responsible for the dead code I removed in #2218 so it might be worth checking that out.

@RGExpert RGExpert requested a review from a team as a code owner October 25, 2025 06:45
@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 25, 2025

@Kuuuube
Copy link
Copy Markdown
Member

Kuuuube commented Oct 25, 2025

I haven't properly looked at the code yet but the reason it was reverted was for ankiconnect android not old pc versions of ankiconnect.

Yomitan can detect if it's running on android if you'd like to use that as a predictor for which api call to try first to avoid sending a unsupported call to ankiconnect android when it is known that Yomitan is running on android.

@RGExpert RGExpert force-pushed the refactor/duplication-logic branch from f7f08b8 to 62fd442 Compare October 25, 2025 07:57
@Manhhao
Copy link
Copy Markdown

Manhhao commented Oct 27, 2025

canAddNotesWithErrorDetail was implemented a while ago in Ankiconnect Android, so it should be supported (should probably be tested to be sure). KamWithK/AnkiconnectAndroid#60

@Kuuuube
Copy link
Copy Markdown
Member

Kuuuube commented Oct 27, 2025

Thank you for the clarification. Sorry for the confusion. This can just go back to trying canAddNotesWithErrorDetail by default and falling back otherwise in that case.

@Kuuuube Kuuuube added kind/enhancement The issue or PR is a new feature or request area/anki The issue or PR is related to Anki integration labels Oct 27, 2025
@RGExpert RGExpert force-pushed the refactor/duplication-logic branch from 62fd442 to 9b1ff24 Compare October 28, 2025 08:05
Copy link
Copy Markdown
Member

@Kuuuube Kuuuube left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you for the contribution!

@Kuuuube Kuuuube added this pull request to the merge queue Oct 28, 2025
Merged via the queue into yomidevs:master with commit 1eecf11 Oct 28, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/anki The issue or PR is related to Anki integration kind/enhancement The issue or PR is a new feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants