-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
keep workbench color customizations when using status bar color #2122
keep workbench color customizations when using status bar color #2122
Conversation
src/mode/modeHandler.ts
Outdated
@@ -2065,11 +2065,11 @@ export class ModeHandler implements vscode.Disposable { | |||
setStatusBarColor(color: string): void { | |||
vscode.workspace.getConfiguration('workbench').update( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider separating the get and the update into two separate lines for readability.
With this change, it also means that if you have workbench configuration, the As the Soliciting @zelphir and @thousand opinions as they both filed related issues (#2150, #1654). Would this fix satisfy your ask? |
@jpoon looks good to me! I would expect that the plugin's behavior could overwrite or replace the behavior of the related configs. |
src/mode/modeHandler.ts
Outdated
vscode.workspace.getConfiguration('workbench').update( | ||
'colorCustomizations', | ||
{ | ||
Object.assign({}, currentColorCustomizations, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've never used Object.assign
before, is there a difference between:
Object.assign({}, currentColor....)
andObject.assign(currentColor...,
ie. why do you need the initial {}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first parameter of the Object.assign
function is the target object. Like you wrote below, it works like an upsert, where from left to right properties get merged.
This is the behavior that I intend, since I want to get a copy of the color customizations and then overwrite its properties regarding the status bar.
The target object gets modified by the Object.assign
call, that's why I pass an empty object at first. But indeed, in this case, this is not needed. I will remove it.
Actually, I misinterpreted the behaviour of That being said, that means we'll be overwriting some settings at the |
Thanks @rodrigo-garcia-leon |
No problem! 😄 |
When the status bar color is controlled by the current mode, the workbench color customizations get overwritten by the status bar customizations, removing any previous existing customization. This change merges both existing and status bar customizations to avoid this behavior.