Finalize setting property and event APIs

[ca-d]

What we have now:
Non-extensible Settings mixin that saves specific settings and *.nsdoc files to the localStorage and shows settings saved to the localStorage in setting dialog.

What we would like to add:
Allow plugins to register their own settings using an event based API

interface RegisterSettingEventDetail {
   name: string,
   translations: Record<BCP47LanguageTag, string>,
   defaultValue: boolean | string,
   kind: "boolean" | "url" | ... | string[],
}

interface SetSettingEventDetail {
   name: string,
   value: boolean | string,
}

Each plugin then gets a settings property set.

interface Settings {
   language: 'en' | 'de',
   ...,
   darkMode: boolean,
   custom: Partial<Record<string,boolean | string>>,
}

@property()
settings: Settings;

[ca-d]

Sharing settings across plugins is impossible with the first draft of this API. We'll need to come up with a system which avoids accidental key collisions between unrelated plugins while allowing related plugins to share user settings between them.

[ca-d]

We would like to resolve this issue by introducing the concept of "plugin sets" (see #17).

When a new setting is registered, it is added (with its default value) to the settings for the plugin set containing the registering plugin. The custom record in the settings property of a plugin contains all settings registered with the plugin's set.

The settings dialogue might then look something like this:

[danyill]

At risk of speaking where I do not understand 😊

I thought I'd look at (for the comparison) the VS Code API for extensions and provide a few ad hoc thoughts. If this is not a useful comparison, please just keep moving, no reply is required 😁

This API looks like:

        /**
         * Get a workspace configuration object.
         *
         * When a section-identifier is provided only that part of the configuration
         * is returned. Dots in the section-identifier are interpreted as child-access,
         * like `{ myExt: { setting: { doIt: true }}}` and `getConfiguration('myExt.setting').get('doIt') === true`.
         *
         * When a scope is provided configuration confined to that scope is returned. Scope can be a resource or a language identifier or both.
         *
         * @param section A dot-separated identifier.
         * @param scope A scope for which the configuration is asked for.
         * @return The full configuration or a subset.
         */
        export function getConfiguration(section?: string, scope?: ConfigurationScope | null): WorkspaceConfiguration;

        /**
         * An event that is emitted when the {@link WorkspaceConfiguration configuration} changed.
         */
        export const onDidChangeConfiguration: Event<ConfigurationChangeEvent>;

I like the idea of:

• Being able to namespace settings and use dots for scope.
• I like the name onDidChangeConfiguration better than SetSettingEventDetail ;-)

This is used in the following way where (asciidoc is the plugin name). I think uniqueness is here enforced by the VS Code marketplace. We could perhaps have a Java style org.openscd or org.lfenergy or org.danyill identifier or just use the plugin URL (or a pseudo, namespace-like URL).

const useEditorStylesheet = vscode.workspace.getConfiguration('asciidoc', null).get('preview.useEditorStyle', false)

This might be overkill for what we need here. In VS Code, extensions have a configuration of all settings provided in package.json, e.g.

		"configuration": {
			"type": "object",
			"title": "asciidoc",
			"order": 21,
			"properties": {
				"asciidoc.asciidoctorpdf_command": {
					"type": "string",
					"default": "asciidoctor-pdf",
					"description": "%asciidoc.asciidoctorpdf_command.desc%"
				},
				"asciidoc.previewFrontMatter": {
					"type": "string",
					"enum": [
						"hide",
						"show"
					],
					"default": "hide",
					"description": "%asciidoc.previewFrontMatter.desc%",
					"scope": "resource"
				},
				"asciidoc.preview.style": {
					"type": "string",
					"default": "",
					"description": "%asciidoc.preview.style.desc%",
					"scope": "resource"

The description and the key for each setting are then able to be used for i18n by providing a json file where the key for each setting and the description field can be provided in different languages.

https://github.com/microsoft/vscode-extension-samples/blob/main/i18n-sample/i18n/jpn/package.i18n.json

I guess I like the declarative style of putting this in the package.json and being able to separately manage i18n.

But my understanding and experience of these things is limited.

When a new setting is registered, it is added (with its default value) to the settings for the plugin set containing the registering plugin. The custom record in the settings property of a plugin contains all settings registered with the plugin's set.

I like the idea of plugin sets.
I am not so confident about associating settings with plugin sets. That seems over-coupled to me.

• Plugin sets are mechanism to enable/disable functionality and groups of functionality.
• Settings are a way to gain configuration and end-user information for individual plugins.
• Plugin sets don't have settings. Individual plugins can simply read the settings of others.

I would ask why shouldn't each plugin be able to declare its own namespace and access settings from any other plugin?

[ca-d]

I thought I'd look at (for the comparison) the VS Code API for extensions and provide a few ad hoc thoughts. If this is not a useful comparison, please just keep moving, no reply is required

Thank you, @danyill , for your extensive research and creative input. There actually is a little bit of misunderstanding baked into your assumptions that I'd like to clear up, but you never know ahead of time, so please keep the feedback coming!

Being able to namespace settings and use dots for scope

You can use dots for scope in your settings if you wish, i18n is a more serious issue for us since we currently intend to allow each plugin to choose its own localisation technology, something we would not be able to do for the settings. This requires some thinking.

Having a global namespace for all settings is something we are desperately trying to avoid, because it's really important to us that different plugins don't read or overwrite each other's settings by accident. Authors of unrelated plugins should not need to coordinate in order to avoid key collisions. For sets of plugins which need to share settings between themselves, we've come up with the solution of plugin sets (see below).

I like the name onDidChangeConfiguration better than SetSettingEventDetail

Actually, the first names the event handler (which we'd need as well as an implementation detail; ours would be called something like onSetSetting or similar), and the second refers to the detail property of the SetSettingEvent itself that is handled by the handler. So I'd prefer we stick to our established naming scheme for that one.

Our way of telling the plugins themselves about changed data is more declarative than that, we simply reactively update the settings property on the plugin component (see below).

I guess I like the declarative style of putting this in the package.json and being able to separately manage i18n.

We really like to use declarative style where possible as well. Currently we intend to declare all plugin metadata in the plugins.json files containing the plugin sets. On the other hand:

• It's important to us that there be a way to manually add a plugin to an existing plugin set without any coordination with the plugin set's authors, in order to enable e.g. third party CoMPAS plugins (not written by the CoMPAS authors) to read and set CoMPAS settings where needed, without needing to coordinate with the CoMPAS project.
• We like the simplicity of having the entry point for the plugin be just a URL to a JavaScript file.
• We also like the consistency of using the same information transport mechanism (reactively setting properties downwards from OpenSCD to the plugins and propagating events upwards from the plugins) everywhere in the system.

These points are of course all up for debate, but that was our reasoning for doing what we've decided to do right now.

This also explains why we introduced plugin sets:

Plugin sets are mechanism to enable/disable functionality and groups of functionality.

We could achieve this with simpler mechanisms as well, but sharing settings both safely and under end-user control was actually at the core of why we came up with plugin sets. In the example given there, two instances of sets of plugins with identical source code (the CoMPAS plugin set) are included from different locations with different plugin set names, in order to allow each of the sets to access a different type of database at a different location and do so consistently. We may then also group the related plugins visually in the menu and maybe even in the tab bar.

I hope this clears up the most important points. If there are still questions or I didn't understand something about your proposal, please feel free to dig deeper, and most importantly:

Please keep the valuable feedback coming ❣️

The ideas you bring in can always inspire new solutions to known problems or even uncover previously unknown problems with current design, and even clearing up misunderstandings like we're doing here will be very helpful for other people in the future trying to understand our reasoning process. Your input truly is valuable either way!

[danyill]

👏 Thanks @ca-d for taking the time to provide detailed explanations, I shall look forward to trialling the new thing when it arrives.

[ca-d]

In last week's sprint change meeting we decided to move to a model where both the "Extensions" plugin management dialog and the "Settings" dialog will be implemented as plugins themselves, thereby enabling "plugin sets/groups" to come with their own settings dialog plugin, obviating the need for a central setting registry altogether. See #17 .