FR: JS API function to remove a specific tag #17716
Comments
|
Working on this! |
|
would suggest implementing "setTags", add tag is a special case of full tag set manipulation, as is remove tag |
|
Oh shoot, just saw this. Your right it would be better, I have a patch like 99% ready with just the unit test for the issue left. Should I scrap it or refactor later, because tbh I can't find where setTags is actually implemented. Are you referring to this? |
|
Unknown? I was mostly making a comment about API design. just seems better to just cut to the chase and treat them as a set with a "set update" operation that could modify the set I think the real lesson here is: beginning to implement a feature request prior to discussing it or seeing it marked as accepted is probably moving just a hair too quickly... any code is a maintenance burden so should be considered carefully prior to addition |
|
Got it, seemed pretty simple that was why I got into patchwork pretty fast. Also yeah I should have waited until it was accepted, sorry about that! will be traveling for the next couple of days so might not be able to pick it up. |
Ok so regarding the API if we use setTags, since it takes an Array of strings how would we remove from the existing set? a pop() function? So we could have a function for set tags and callable methods like remove, add and set? Because it might be more easy to use then raw set operations. |
|
@mikehardy Can I start working on refactoring the add tag into a setTags API function? |
|
I think set tags with args as noteId, tags, operation (set, remove, add) is better, any comments? |
|
I prefer REST style APIs personally, which argues for 2 APIs only, "get tags for this thing" and "set tags for this thing". The tags as a set are a "document" in a REST API style. Caller handles the set mutation to do whatever they want |
|
ohh makes sense, ok then! |
|
@mikehardy what do you think of this prelim patch? I wanna get some feedback b4 going for a PR or building testcases. Subject: [PATCH] Unit Tests for initializeApkgExportUi and initializeNotesExportUi
---
Index: AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt b/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt (revision ae3e639c535653b2365adfff61464a67c772386e)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt (date 1736447518066)
@@ -373,6 +373,29 @@
getColUnsafe.updateNote(note)
convertToByteArray(apiContract, true)
}
+
+ "setTagsToNote" -> {
+ val jsonObject = JSONObject(apiParams)
+ val noteId = jsonObject.getLong("noteId")
+ val tags = jsonObject.getJSONArray("tags")
+ val tagsList = mutableListOf<String>()
+ for (i in 0 until tags.length()) {
+ tagsList.add(tags.getString(i))
+ }
+ val note = getColUnsafe.getNote(noteId).apply {
+ setTagsFromStr(getColUnsafe, tagsList.joinToString(","))
+ }
+ getColUnsafe.updateNote(note)
+ convertToByteArray(apiContract, true)
+ }
+
+ "getTagsFromNote" -> {
+ val jsonObject = JSONObject(apiParams)
+ val noteId = jsonObject.getLong("noteId")
+ val note = getColUnsafe.getNote(noteId)
+ convertToByteArray(apiContract, note.tags.joinToString(","))
+ }
+
"sttSetLanguage" -> convertToByteArray(apiContract, speechRecognizer.setLanguage(apiParams))
"sttStart" -> {
val callback =
Index: AnkiDroid/src/main/assets/scripts/js-api.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/assets/scripts/js-api.js b/AnkiDroid/src/main/assets/scripts/js-api.js
--- a/AnkiDroid/src/main/assets/scripts/js-api.js (revision ae3e639c535653b2365adfff61464a67c772386e)
+++ b/AnkiDroid/src/main/assets/scripts/js-api.js (date 1736447299376)
@@ -72,6 +72,8 @@
ankiSttStart: "sttStart",
ankiSttStop: "sttStop",
ankiAddTagToNote: "addTagToNote",
+ ankiSetTagsToNote: "setTagsToNote",
+ ankiGetTagsFromNote: "getTagsFromNote",
};
class AnkiDroidJS {
@@ -125,6 +127,21 @@
};
return;
}
+ if (method === "ankiSetTagsToNote") {
+ AnkiDroidJS.prototype[method] = async function (noteId, tags) {
+ const endpoint = jsApiList[method];
+ const data = JSON.stringify({ noteId, tags});
+ return await this.handleRequest(endpoint, data);
+ };
+ return;
+ }
+ if (method === "ankiGetTagsFromNote") {
+ AnkiDroidJS.prototype[method] = async function (noteId) {
+ const endpoint = jsApiList[method];
+ const data = JSON.stringify({ noteId });
+ return await this.handleRequest(endpoint, data);
+ }
+ }
if (method === "ankiTtsSpeak") {
AnkiDroidJS.prototype[method] = async function (text, queueMode = 0) {
const endpoint = jsApiList[method]; |
This comment has been minimized.
This comment has been minimized.
|
@silidev while this is not what you exactly asked for, are you fine with this? |
|
This is verbose, and should probably use the val tagsList = mutableListOf<String>()
for (i in 0 until tags.length()) {
tagsList.add(tags.getString(i))
}
val note = getColUnsafe.getNote(noteId).apply {
setTagsFromStr(getColUnsafe, tagsList.joinToString(","))
}could use the following. But tags.stringIterable().joinToString(",") |
|
Yes, great! No need to ask me though. David decides.
--
Viele Grüße - Helge
Hari Srinivasan ***@***.***> schrieb am Do., 9. Jan. 2025,
19:58:
… @silidev <https://github.com/silidev> while this is not what you exactly
asked for, are you fine with this?
—
Reply to this email directly, view it on GitHub
<#17716 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AD2V77SCWPKTZM27TPCSTCT2J3BDXAVCNFSM6AAAAABUTIDFEGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKOBRGA2DCNJVGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@silidev You asked for it, it's very useful to know if it's fit for purpose! |
|
OK, thank you, anything that lets me remove a tag works for me.
|
Subject: [PATCH] Unit Tests for initializeApkgExportUi and initializeNotesExportUi
---
Index: AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt b/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt (revision ae3e639c535653b2365adfff61464a67c772386e)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt (date 1736506352662)
@@ -47,12 +47,14 @@
import com.ichi2.libanki.Decks
import com.ichi2.libanki.SortOrder
import com.ichi2.utils.NetworkUtils
+import com.ichi2.utils.stringIterable
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import kotlinx.serialization.builtins.ListSerializer
import kotlinx.serialization.builtins.serializer
import kotlinx.serialization.json.Json
+import org.json.JSONArray
import org.json.JSONException
import org.json.JSONObject
import timber.log.Timber
@@ -373,6 +375,25 @@
getColUnsafe.updateNote(note)
convertToByteArray(apiContract, true)
}
+
+ "[set/replace]NoteTags" -> {
+ val jsonObject = JSONObject(apiParams)
+ val noteId = jsonObject.getLong("noteId")
+ val tags = jsonObject.getJSONArray("tags")
+ val note = getColUnsafe.getNote(noteId).apply {
+ setTagsFromStr(getColUnsafe, tags.stringIterable().joinToString(","))
+ }
+ getColUnsafe.updateNote(note)
+ convertToByteArray(apiContract, true)
+ }
+
+ "getTagsFromNote" -> {
+ val jsonObject = JSONObject(apiParams)
+ val noteId = jsonObject.getLong("noteId")
+ val note = getColUnsafe.getNote(noteId)
+ convertToByteArray(apiContract, JSONArray(note.tags).toString())
+ }
+
"sttSetLanguage" -> convertToByteArray(apiContract, speechRecognizer.setLanguage(apiParams))
"sttStart" -> {
val callback =
Index: AnkiDroid/src/main/assets/scripts/js-api.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/assets/scripts/js-api.js b/AnkiDroid/src/main/assets/scripts/js-api.js
--- a/AnkiDroid/src/main/assets/scripts/js-api.js (revision ae3e639c535653b2365adfff61464a67c772386e)
+++ b/AnkiDroid/src/main/assets/scripts/js-api.js (date 1736506121908)
@@ -72,6 +72,8 @@
ankiSttStart: "sttStart",
ankiSttStop: "sttStop",
ankiAddTagToNote: "addTagToNote",
+ anki[set/replace]NoteTags: "[set/replace]NoteTags",
+ ankiGetTagsFromNote: "getTagsFromNote",
};
class AnkiDroidJS {
@@ -125,6 +127,21 @@
};
return;
}
+ if (method === "anki[set/replace]NoteTags") {
+ AnkiDroidJS.prototype[method] = async function (noteId, tags) {
+ const endpoint = jsApiList[method];
+ const data = JSON.stringify({ noteId, tags});
+ return await this.handleRequest(endpoint, data);
+ };
+ return;
+ }
+ if (method === "ankiGetTagsFromNote") {
+ AnkiDroidJS.prototype[method] = async function (noteId) {
+ const endpoint = jsApiList[method];
+ const data = JSON.stringify({ noteId });
+ return await this.handleRequest(endpoint, data);
+ }
+ }
if (method === "ankiTtsSpeak") {
AnkiDroidJS.prototype[method] = async function (text, queueMode = 0) {
const endpoint = jsApiList[method];@david-allison Good to go? Btw I don't get how the Tags class can be used, are you referring to Tags.kt in libanki? |
|
@Haz3-jolt easier to review in a pull request. Let's move this forwards |
This is a sister issue to the already released issue #16210 (comment) , just removing a tag instead of adding it.
@Haz3-jolt will implement this (source: #16210 (comment) )
The text was updated successfully, but these errors were encountered: