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

cache clipboard image to prevent duplicating in anki media #776

Merged
merged 7 commits into from
Apr 18, 2024

Conversation

StefanVukovic99
Copy link
Member

Resolves #767

Since the same filename would now be used on different cards it shouldn't contain the term/reading, as that would mess with searching in anki.

@StefanVukovic99 StefanVukovic99 requested a review from a team as a code owner March 12, 2024 21:20
Copy link

github-actions bot commented Mar 12, 2024

✔️ No visual differences introduced by this PR.

View Playwright Report (note: open the "playwright-report" artifact)

@StefanVukovic99 StefanVukovic99 added area/anki The issue or PR is related to Anki integration kind/enhancement The issue or PR is a new feature or request labels Mar 13, 2024
* @returns {string}
*/
_generateAnkiNoteMediaFileName(prefix, extension, timestamp, definitionDetails) {
_generateAnkiNoteMediaFileName(prefix, extension, timestamp, definitionDetails = null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Accepting null here (and actually using it) means that different definitions could result with the same filename if they are looked up in the same second, right? Can this cause problems?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hadn't considered that. If someone did

  1. copy image A
  2. add term A
  3. copy image B
  4. add term B

and 2-4 happened within the same second, image B would unexpectedly overwrite image A in anki's media folder.

It's pretty hard to do it that fast, but a couple ways to prevent it:

  • Add more digits to the timestamp
  • Generate a UID and either append it, or replace the timestamp
  • Use deleteExisting: false in the ankiconnect call. Anki would then not overwrite files on filename. Would require another AnkiConnect call to check if a file is already added.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, I guess using a more precise timestamp is the simplest solution.

BTW, is there any point in keeping the definitionDetails input at all? Wouldn't it be simpler to just eliminate it instead of creating a branch in the logic?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I guess with the more precise timestamp, the term/reading isn't needed in any of the filenames.

@jamesmaa jamesmaa added this pull request to the merge queue Apr 18, 2024
Merged via the queue into themoeway:master with commit 8f258c0 Apr 18, 2024
10 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.

Stop duplicating clipboard image in anki
3 participants