From a7b4c9014652ecdab00f1cb6b270b7714b8f932c Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Sun, 17 Sep 2023 15:21:20 +1000 Subject: [PATCH] Use field tags instead of hard-coding occlusion fields + Don't protect the comments field It's not required by our current code. We can remove the protection from Header and Back Extra in the future too, once we no longer depend on them. Closes #2621 --- .eslintrc.js | 2 +- ftl/core/notetypes.ftl | 1 + proto/anki/image_occlusion.proto | 27 ++++++-- qt/aqt/addcards.py | 2 +- qt/aqt/mediasrv.py | 1 + rslib/src/dbcheck.rs | 3 +- rslib/src/image_occlusion/imagedata.rs | 87 +++++++++++++++++++------- rslib/src/image_occlusion/notetype.rs | 2 +- rslib/src/image_occlusion/service.rs | 36 ++++++----- ts/editor/NoteEditor.svelte | 31 +++++---- 10 files changed, 134 insertions(+), 58 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index f18b7ea61..01dc09d52 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -49,7 +49,7 @@ module.exports = { }, }, ], - env: { browser: true }, + env: { browser: true, es2020: true }, ignorePatterns: ["backend_proto.d.ts", "*.svelte.d.ts", "vendor", "extra/*"], globals: { globalThis: false, diff --git a/ftl/core/notetypes.ftl b/ftl/core/notetypes.ftl index 96f4087e3..403800624 100644 --- a/ftl/core/notetypes.ftl +++ b/ftl/core/notetypes.ftl @@ -52,4 +52,5 @@ notetypes-error-getting-imagecloze = An error occurred while fetching an image o notetypes-error-loading-image-occlusion = Error loading image occlusion. Is your Anki version up to date? notetype-error-no-image-to-show = No image to show. notetypes-no-occlusion-created = You must make at least one occlusion. +notetypes-no-occlusion-created2 = Unable to add. Either you have not added any occlusions, or the first field is empty. notetypes-io-select-image = Select Image diff --git a/proto/anki/image_occlusion.proto b/proto/anki/image_occlusion.proto index 6bea48569..cd600c7c0 100644 --- a/proto/anki/image_occlusion.proto +++ b/proto/anki/image_occlusion.proto @@ -13,14 +13,18 @@ import "anki/generic.proto"; service ImageOcclusionService { rpc GetImageForOcclusion(GetImageForOcclusionRequest) returns (GetImageForOcclusionResponse); - rpc AddImageOcclusionNote(AddImageOcclusionNoteRequest) - returns (collection.OpChanges); rpc GetImageOcclusionNote(GetImageOcclusionNoteRequest) returns (GetImageOcclusionNoteResponse); - rpc UpdateImageOcclusionNote(UpdateImageOcclusionNoteRequest) - returns (collection.OpChanges); + rpc GetImageOcclusionFields(GetImageOcclusionFieldsRequest) + returns (GetImageOcclusionFieldsResponse); // Adds an I/O notetype if none exists in the collection. rpc AddImageOcclusionNotetype(generic.Empty) returns (collection.OpChanges); + // These two are used by the standalone I/O page, but not used when using + // I/O inside Anki's editor + rpc AddImageOcclusionNote(AddImageOcclusionNoteRequest) + returns (collection.OpChanges); + rpc UpdateImageOcclusionNote(UpdateImageOcclusionNoteRequest) + returns (collection.OpChanges); } // Implicitly includes any of the above methods that are not listed in the @@ -71,3 +75,18 @@ message UpdateImageOcclusionNoteRequest { string back_extra = 4; repeated string tags = 5; } + +message GetImageOcclusionFieldsRequest { + int64 notetype_id = 1; +} + +message GetImageOcclusionFieldsResponse { + ImageOcclusionFieldIndexes fields = 1; +} + +message ImageOcclusionFieldIndexes { + uint32 occlusions = 1; + uint32 image = 2; + uint32 header = 3; + uint32 back_extra = 4; +} diff --git a/qt/aqt/addcards.py b/qt/aqt/addcards.py index d39927cc2..3ccc71731 100644 --- a/qt/aqt/addcards.py +++ b/qt/aqt/addcards.py @@ -305,7 +305,7 @@ class AddCards(QMainWindow): problem = None if result == NoteFieldsCheckResult.EMPTY: if self.editor.current_notetype_is_image_occlusion(): - problem = tr.notetypes_no_occlusion_created() + problem = tr.notetypes_no_occlusion_created2() else: problem = tr.adding_the_first_field_is_empty() elif result == NoteFieldsCheckResult.MISSING_CLOZE: diff --git a/qt/aqt/mediasrv.py b/qt/aqt/mediasrv.py index 5988dcb25..e50f15dfe 100644 --- a/qt/aqt/mediasrv.py +++ b/qt/aqt/mediasrv.py @@ -544,6 +544,7 @@ exposed_backend_list = [ "add_image_occlusion_note", "get_image_occlusion_note", "update_image_occlusion_note", + "get_image_occlusion_fields", # SchedulerService "compute_fsrs_weights", "compute_optimal_retention", diff --git a/rslib/src/dbcheck.rs b/rslib/src/dbcheck.rs index be96029b0..4e26d0da7 100644 --- a/rslib/src/dbcheck.rs +++ b/rslib/src/dbcheck.rs @@ -6,6 +6,7 @@ use std::sync::Arc; use anki_i18n::I18n; use anki_proto::notetypes::stock_notetype::OriginalStockKind; +use anki_proto::notetypes::ImageOcclusionField; use itertools::Itertools; use tracing::debug; @@ -442,7 +443,7 @@ impl Collection { let conf = &mut nt.fields[i].config; if !conf.prevent_deletion { changed = true; - conf.prevent_deletion = true; + conf.prevent_deletion = i != ImageOcclusionField::Comments as usize; conf.tag = Some(i as u32); } } diff --git a/rslib/src/image_occlusion/imagedata.rs b/rslib/src/image_occlusion/imagedata.rs index 3035d3b52..f484b11f8 100644 --- a/rslib/src/image_occlusion/imagedata.rs +++ b/rslib/src/image_occlusion/imagedata.rs @@ -8,8 +8,11 @@ use anki_io::metadata; use anki_io::read_file; use anki_proto::image_occlusion::get_image_occlusion_note_response::ImageClozeNote; use anki_proto::image_occlusion::get_image_occlusion_note_response::Value; +use anki_proto::image_occlusion::AddImageOcclusionNoteRequest; use anki_proto::image_occlusion::GetImageForOcclusionResponse; use anki_proto::image_occlusion::GetImageOcclusionNoteResponse; +use anki_proto::image_occlusion::ImageOcclusionFieldIndexes; +use anki_proto::notetypes::ImageOcclusionField; use regex::Regex; use crate::media::MediaManager; @@ -24,19 +27,13 @@ impl Collection { Ok(metadata) } - #[allow(clippy::too_many_arguments)] pub fn add_image_occlusion_note( &mut self, - notetype_id: NotetypeId, - image_path: &str, - occlusions: &str, - header: &str, - back_extra: &str, - tags: Vec, + req: AddImageOcclusionNoteRequest, ) -> Result> { // image file - let image_bytes = read_file(image_path)?; - let image_filename = Path::new(&image_path) + let image_bytes = read_file(&req.image_path)?; + let image_filename = Path::new(&req.image_path) .file_name() .or_not_found("expected filename")? .to_str() @@ -49,6 +46,7 @@ impl Collection { let image_tag = format!(r#""#, &actual_image_name_after_adding); let current_deck = self.get_current_deck()?; + let notetype_id: NotetypeId = req.notetype_id.into(); self.transact(Op::ImageOcclusion, |col| { let nt = if notetype_id.0 == 0 { // when testing via .html page, use first available notetype @@ -60,11 +58,12 @@ impl Collection { }; let mut note = nt.new_note(); - note.set_field(0, occlusions)?; - note.set_field(1, &image_tag)?; - note.set_field(2, header)?; - note.set_field(3, back_extra)?; - note.tags = tags; + let idxs = nt.get_io_field_indexes()?; + note.set_field(idxs.occlusions as usize, req.occlusions)?; + note.set_field(idxs.image as usize, image_tag)?; + note.set_field(idxs.header as usize, req.header)?; + note.set_field(idxs.back_extra as usize, req.back_extra)?; + note.tags = req.tags; col.add_note_inner(&mut note, current_deck.id)?; Ok(()) @@ -87,17 +86,19 @@ impl Collection { let mut cloze_note = ImageClozeNote::default(); let fields = note.fields(); - if fields.len() < 4 { - invalid_input!("Note does not have 4 fields"); - } - cloze_note.occlusions = fields[0].clone(); - cloze_note.header = fields[2].clone(); - cloze_note.back_extra = fields[3].clone(); + let nt = self + .get_notetype(note.notetype_id)? + .or_not_found(note.notetype_id)?; + let idxs = nt.get_io_field_indexes()?; + + cloze_note.occlusions = fields[idxs.occlusions as usize].clone(); + cloze_note.header = fields[idxs.header as usize].clone(); + cloze_note.back_extra = fields[idxs.back_extra as usize].clone(); cloze_note.image_data = "".into(); cloze_note.tags = note.tags.clone(); - let image_file_name = &fields[1]; + let image_file_name = &fields[idxs.image as usize]; let src = self .extract_img_src(image_file_name) .unwrap_or_else(|| "".to_owned()); @@ -120,9 +121,13 @@ impl Collection { ) -> Result> { let mut note = self.storage.get_note(note_id)?.or_not_found(note_id)?; self.transact(Op::ImageOcclusion, |col| { - note.set_field(0, occlusions)?; - note.set_field(2, header)?; - note.set_field(3, back_extra)?; + let nt = col + .get_notetype(note.notetype_id)? + .or_not_found(note.notetype_id)?; + let idxs = nt.get_io_field_indexes()?; + note.set_field(idxs.occlusions as usize, occlusions)?; + note.set_field(idxs.header as usize, header)?; + note.set_field(idxs.back_extra as usize, back_extra)?; note.tags = tags; col.update_note_inner(&mut note)?; Ok(()) @@ -156,3 +161,37 @@ impl Collection { Ok(false) } } + +impl Notetype { + pub(crate) fn get_io_field_indexes(&self) -> Result { + get_field_indexes_by_tag(self).or_else(|_| { + if self.fields.len() < 4 { + return Err(AnkiError::DatabaseCheckRequired); + } + Ok(ImageOcclusionFieldIndexes { + occlusions: 0, + image: 1, + header: 2, + back_extra: 3, + }) + }) + } +} + +fn get_field_indexes_by_tag(nt: &Notetype) -> Result { + Ok(ImageOcclusionFieldIndexes { + occlusions: get_field_index(nt, ImageOcclusionField::Occlusions)?, + image: get_field_index(nt, ImageOcclusionField::Image)?, + header: get_field_index(nt, ImageOcclusionField::Header)?, + back_extra: get_field_index(nt, ImageOcclusionField::BackExtra)?, + }) +} + +fn get_field_index(nt: &Notetype, field: ImageOcclusionField) -> Result { + nt.fields + .iter() + .enumerate() + .find(|(_idx, f)| f.config.tag == Some(field as u32)) + .map(|(idx, _)| idx as u32) + .ok_or(AnkiError::DatabaseCheckRequired) +} diff --git a/rslib/src/image_occlusion/notetype.rs b/rslib/src/image_occlusion/notetype.rs index b889eb7f8..0f94c5e6d 100644 --- a/rslib/src/image_occlusion/notetype.rs +++ b/rslib/src/image_occlusion/notetype.rs @@ -86,7 +86,7 @@ pub(crate) fn image_occlusion_notetype(tr: &I18n) -> Notetype { let comments = tr.notetypes_comments_field(); config = nt.add_field(comments.as_ref()); config.tag = Some(ImageOcclusionField::Comments as u32); - config.prevent_deletion = true; + config.prevent_deletion = false; let err_loading = tr.notetypes_error_loading_image_occlusion(); let qfmt = format!( diff --git a/rslib/src/image_occlusion/service.rs b/rslib/src/image_occlusion/service.rs index 308eef0c3..ee17d074c 100644 --- a/rslib/src/image_occlusion/service.rs +++ b/rslib/src/image_occlusion/service.rs @@ -3,47 +3,42 @@ use anki_proto::image_occlusion::AddImageOcclusionNoteRequest; use anki_proto::image_occlusion::GetImageForOcclusionRequest; use anki_proto::image_occlusion::GetImageForOcclusionResponse; +use anki_proto::image_occlusion::GetImageOcclusionFieldsRequest; +use anki_proto::image_occlusion::GetImageOcclusionFieldsResponse; use anki_proto::image_occlusion::GetImageOcclusionNoteRequest; use anki_proto::image_occlusion::GetImageOcclusionNoteResponse; use anki_proto::image_occlusion::UpdateImageOcclusionNoteRequest; use crate::collection::Collection; -use crate::error; +use crate::error::Result; +use crate::prelude::*; impl crate::services::ImageOcclusionService for Collection { fn get_image_for_occlusion( &mut self, input: GetImageForOcclusionRequest, - ) -> error::Result { + ) -> Result { self.get_image_for_occlusion(&input.path) } fn add_image_occlusion_note( &mut self, input: AddImageOcclusionNoteRequest, - ) -> error::Result { - self.add_image_occlusion_note( - input.notetype_id.into(), - &input.image_path, - &input.occlusions, - &input.header, - &input.back_extra, - input.tags, - ) - .map(Into::into) + ) -> Result { + self.add_image_occlusion_note(input).map(Into::into) } fn get_image_occlusion_note( &mut self, input: GetImageOcclusionNoteRequest, - ) -> error::Result { + ) -> Result { self.get_image_occlusion_note(input.note_id.into()) } fn update_image_occlusion_note( &mut self, input: UpdateImageOcclusionNoteRequest, - ) -> error::Result { + ) -> Result { self.update_image_occlusion_note( input.note_id.into(), &input.occlusions, @@ -54,7 +49,18 @@ impl crate::services::ImageOcclusionService for Collection { .map(Into::into) } - fn add_image_occlusion_notetype(&mut self) -> error::Result { + fn add_image_occlusion_notetype(&mut self) -> Result { self.add_image_occlusion_notetype().map(Into::into) } + + fn get_image_occlusion_fields( + &mut self, + input: GetImageOcclusionFieldsRequest, + ) -> Result { + let ntid = NotetypeId::from(input.notetype_id); + let nt = self.get_notetype(ntid)?.or_not_found(ntid)?; + Ok(GetImageOcclusionFieldsResponse { + fields: Some(nt.get_io_field_indexes()?), + }) + } } diff --git a/ts/editor/NoteEditor.svelte b/ts/editor/NoteEditor.svelte index 2a62ab9ff..fba0fc520 100644 --- a/ts/editor/NoteEditor.svelte +++ b/ts/editor/NoteEditor.svelte @@ -267,7 +267,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html fontSize: fonts[index][1], direction: fonts[index][2] ? "rtl" : "ltr", collapsed: fieldsCollapsed[index], - hidden: hideFieldInOcclusionType(index), + hidden: hideFieldInOcclusionType(index, ioFields), })) as FieldData[]; function saveTags({ detail }: CustomEvent): void { @@ -384,6 +384,8 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html }); } + import { ImageOcclusionFieldIndexes } from "@tslib/anki/image_occlusion_pb"; + import { getImageOcclusionFields } from "@tslib/backend"; import { wrapInternal } from "@tslib/wrap"; import LabelButton from "components/LabelButton.svelte"; import Shortcut from "components/Shortcut.svelte"; @@ -398,19 +400,23 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html let isIOImageLoaded = false; let imageOcclusionMode: IOMode | undefined; + let ioFields = new ImageOcclusionFieldIndexes({}); async function setupMaskEditor(options: { html: string; mode: IOMode }) { imageOcclusionMode = undefined; - await tick(); + const getIoFields = getImageOcclusionFields({ + notetypeId: BigInt(notetypeMeta.id), + }).then((r) => (ioFields = r.fields!)); + await Promise.all([tick(), getIoFields]); imageOcclusionMode = options.mode; if (options.mode.kind === "add") { - fieldStores[1].set(options.html); + fieldStores[ioFields.image].set(options.html); // new image is being added if (isIOImageLoaded) { resetIOImage(options.mode.imagePath); } } else { - const clozeNote = get(fieldStores[0]); + const clozeNote = get(fieldStores[ioFields.occlusions]); if (clozeNote.includes("oi=1")) { $hideAllGuessOne = true; } else { @@ -422,14 +428,14 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html } function setImageField(html) { - fieldStores[1].set(html); + fieldStores[ioFields.image].set(html); } globalThis.setImageField = setImageField; // update cloze deletions and set occlusion fields, it call in saveNow to update cloze deletions function updateIONoteInEditMode() { if (isEditMode) { - const clozeNote = get(fieldStores[0]); + const clozeNote = get(fieldStores[ioFields.occlusions]); if (clozeNote.includes("oi=1")) { setOcclusionField(true); } else { @@ -441,7 +447,7 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html function setOcclusionFieldInner() { if (isImageOcclusion) { const occlusionsData = exportShapesToClozeDeletions($hideAllGuessOne); - fieldStores[0].set(occlusionsData.clozes); + fieldStores[ioFields.occlusions].set(occlusionsData.clozes); } } // global for calling this method in desktop note editor @@ -462,14 +468,17 @@ License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html // set fields data for occlusion and image fields for io notes type if (isImageOcclusion) { const occlusionsData = exportShapesToClozeDeletions(occludeInactive); - fieldStores[0].set(occlusionsData.clozes); + fieldStores[ioFields.occlusions].set(occlusionsData.clozes); } } - // hide first two fields for occlusion type, first contains occlusion data and second contains image - function hideFieldInOcclusionType(index: number) { + /** hide occlusions and image */ + function hideFieldInOcclusionType( + index: number, + ioFields: ImageOcclusionFieldIndexes, + ) { if (isImageOcclusion) { - if (index == 0 || index == 1) { + if (index === ioFields.occlusions || index === ioFields.image) { return true; } }