Skip to content

[themes] Add ability to reload themes without restarting the editor - #66115

Merged
Martin Aeschlimann (aeschli) merged 4 commits into
microsoft:masterfrom
JimiC:reload_theme
Jan 21, 2019
Merged

[themes] Add ability to reload themes without restarting the editor#66115
Martin Aeschlimann (aeschli) merged 4 commits into
microsoft:masterfrom
JimiC:reload_theme

Conversation

@JimiC

Copy link
Copy Markdown
Contributor

Resolves #45963

This PRs adds the ability to reload a theme (color or icon) without the need to restart the editor.

Replaces #60136

@aeschli

Copy link
Copy Markdown
Contributor

made some changes.

@JimiC

Copy link
Copy Markdown
Contributor Author

Fine by me as long as it brings us one step closer to merging. 😄

@JimiC

Copy link
Copy Markdown
Contributor Author

Looks like createUnloadedTheme requires undefined for settingsId and extensionData.

@aeschli Martin Aeschlimann (aeschli) changed the title Add ability to reload themes without restarting the editor [themes] Add ability to reload themes without restarting the editor Jan 21, 2019
@aeschli
Martin Aeschlimann (aeschli) merged commit c596383 into microsoft:master Jan 21, 2019
@aeschli

Copy link
Copy Markdown
Contributor

We had long discussions in the team about this feature. By design, the content of extensions are expected to be immutable. Only our extension management code should make changes.
Adding a watcher is the wrong approach. Instead we should add a icon/color theme provider API in code that would allow you to implement a more powerful icon theme.
But that API isn't going to happen very soon, so I added a hidden/ & in-official flag for you where you can turn on a watcher for your theme.

		"iconThemes": [
			{
				"id": "vs-minimal",
				"label": "Minimal (Visual Studio Code)",
				"path": "./fileicons/vs_minimal-icon-theme.json",
				"_watch": true
			}
		]

No warranties on that flag and it might also be removed any time if necessary. But I will let you know once we have the proper API in place.

Thanks for your help!

@JimiC

Dimitris "Jimi" Charalampidis (JimiC) commented Jan 21, 2019

Copy link
Copy Markdown
Contributor Author

So that's what _watch is all about.
Sorry if I'm wrong but I only see this being declared for color themes and not icon themes (ColorThemeData).
Discard the above, I just saw 24e8857

Just for the record, I proposed an API but it was rejected for not fitting to the current API (#60136 (comment)). I'm totally in favor of an API point and that was my original approach. But in order to see the reloading feature happening, I went with whatever was suggested.

I don't want to sound bitter but a temp solution is like no solution at all. Never the less, I'll try to see the cup half-full and not half-empty. Some will say that from not having this at all, at least we can have it as it is.

@aeschli

Copy link
Copy Markdown
Contributor

Yes, sorry, my fault that we went down that pass. But also a reload API doesn't make any sense given that extensions are not supposed to make changes to their content. You guys also only do that to work around limitations on the icon theme syntax.
What you want is a file icon theme provider that lets you return an icon for a given file path. However, that's quite a lot of changes and probably requires us to refactor the way icon associations are made.

@github-actions github-actions Bot locked and limited conversation to collaborators Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[icon themes] Expose API to provide a dynamic icon theme.

2 participants