feat: Labs interface backend

[Luc-Mcgrady]

closes #5073

Saves the values set in

• feat: Labs option in preferences #5057

Saves it to the currently open collections json config. I'm not sure if this approach might have problems with #4289 (comment). A situation where the main window has Anki API access while also displaying user supplied JavaScript would be terrible so we should be careful with this.

Test by enabling the "ping" labs option and then reload Anki or switch profile to the same profile.

[github-actions]

Thanks for the PR! To ensure we're solving the right problems, we require all PRs to be linked to an open issue. Please open one describing the problem this PR addresses so we can discuss the implementation there first.

You have 4 days to link an issue before this PR is automatically closed, it can be reopened at any time after that. Looking forward to the discussion!

To link an issue, edit the PR description and add a line like Closes #123.

[read-the-docs-community]

Documentation build overview

📚 Anki | 🛠️ Build #33372499 | 📁 Comparing dc62d74 against latest (36213cb)

  🔍 Preview build  

3 files changed

± genindex.html
± autoapi/anki/config/index.html
± autoapi/aqt/webview/index.html

[Luc-Mcgrady]

After thinking about this a little more, It would probably be better to have a separate "labs.json" or "global_settings" file that is stored in the root folder of the Anki collection. (not the profile folder) a bit like the graphics driver:

anki/qt/aqt/profiles.py

Lines 525 to 527 in 07c2b03

	def set_video_driver(self, driver: VideoDriver) -> None:
	with open(self._gldriver_path(), "w", encoding="utf8") as file:
	file.write(driver.value)

There is an existing prefs21.db but im not sure about adding a new table and also not sure that it is suited to the labs where optional things may be added and removed quickly.

Any thoughts?

[abdnh]

After thinking about this a little more, It would probably be better to have a separate "labs.json" or "global_settings" file that is stored in the root folder of the Anki collection. (not the profile folder)

What's your reasoning here? I don't feel this warrants a new storage mechanism.

[Luc-Mcgrady]

What's your reasoning here? I don't feel this warrants a new storage mechanism.

I was worried about issues where the experimental changes might be partially loaded, especially in regards to #4289 (comment). If the user loads the profile then there might be a situation where an un-sandboxed main window has API access. However it might be better to leave the pr in its current state until we can confirm that this is an actual problem. Maybe loading a profile will have the same effect as restarting when it comes to the window having API access.

[abdnh]

This is my suggestion based on our discussion if we want to handle it here:

1. Intercept the setConfigJson call as noted above but also update ConfigManager._experiments to reflect the changes.
2. Add an additional check in AuthInterceptor to deny API access to the main window (based on info.requestUrl().path()) if the new reviewer is not enabled.

However it might be better to leave the pr in its current state until we can confirm that this is an actual problem

👍

[abdnh]

@Luc-Mcgrady regarding your comment on the other PR:

Before we can add the functionality we should try and come up with the pattern that we should use to register new flags.

As a simple solution, we could keep a string list in get_config_json() and clean up any keys not listed.

A more type-safe option is to add a Protobuf type similar to ConfigKey.Bool

[Luc-Mcgrady]

As a simple solution, we could keep a string list in get_config_json() and clean up any keys not listed.

A more type-safe option is to add a Protobuf type similar to ConfigKey.Bool

Is there anything wrong with the current get_config_json() {key: bool} setup?

[abdnh]

Is there anything wrong with the current get_config_json() {key: bool} setup?

I was just thinking it'd be easier to remove/add flags safely in the future if all valid keys were declared in a single place. Declaring the keys in Protobuf is to guard against typos or cases where we remove a flag but accidentally leave a usage of it in some place. It's also useful if we plan to use experimental features in other clients such as AnkiMobile.

[Luc-Mcgrady]

I was just thinking it'd be easier to remove/add flags safely in the future if all valid keys were declared in a single place. Declaring the keys in Protobuf is to guard against typos or cases where we remove a flag but accidentally leave a usage of it in some place. It's also useful if we plan to use experimental features in other clients such as AnkiMobile.

Oh I get it now. I'll try and do that. (done dc62d74)

As a simple solution, we could keep a string list in get_config_json() and clean up any keys not listed.

Do you think I should replace the current get_config_json() {key: bool} setup with this though?

[abdnh]

I think this is looking good. Some notes:

1. We're storing the enum integers as JSON keys, which is unusual but fine as long as we don't reuse values. Changing this might require reinventing Config.Bool so let's leave it for now.
2. In the future as we remove flags, we might want to leave them in ExperimentalFeatureFlag and add some special handling so that they are forced to be set/unset. This should guard against cases where we forget to remove old flag usages and is useful if add-ons want to check flags.

[fernandolins]

for the future, we must think of improving Anki so we won't need it anymore

[fernandolins]

regarding this:

There is an existing prefs21.db but im not sure about adding a new table and also not sure that it is suited to the labs where optional things may be added and removed quickly.

Maybe for the future we should use prefs21.db as a permanent database for this experiment, but for now, I agree to keep it simple as it is.