configuration only updates after restart

[Cube707]

🚀 Feature Request: Live update all configurations

Problem:

Currently all configurations are loaded once on startup of the extentions. This means that after a configuration is changed, it will only take effect after a reload. This is slightly anoint when testing settings.

Describe the solution you'd like

For Version 3.0 a callback handler should be implemented that updates the IUserSettings class when the usere chnages the configurations (if that is posible). Or at least the promt "reload window" should apperear.

[Josee9988]

The live update was "disabled" to improve the extension performance (avoid reading the settings before any command was executed).

I think adding that a "reload window" would be great. I have to figure out how to check if any MinifyAll settings have changed and if so check if making these checks is more time consuming than just reading them before any execution.

[Cube707]

This StackOverflow Post looks promising: https://stackoverflow.com/questions/48207461/on-property-change-event-handler

Checking it in the documentations, it's still be alive an kicking: callback workspace.onDidChangeConfiguration and Event ConfigurationChangeEvent

This seems to be exacly whats needed and can even check if certain configurations where effected.

I could implement a testing version of this next week if your busy?

[Josee9988]

You can try it if you want, that would be a pleasure :D If not I could try to do it by the end of October.

Awaiting your response. :)

[Cube707]

I will definitely try to get it to work. Will come back to you if I have run into a git issue or have something thats testable :)

[Josee9988]

Perfect! Let me know if you need any help or anything. I appreciate your effort

[Cube707]

I have two working versions now under https://github.com/Cube707/MinifyAll/tree/realtime-config-updates

First commit is a simple version that allways promts for a reload whenever the config is changed.

The second commit only promts for the two options minifyOnSave and minifyOnSaveToNewFile, which have a callbackhandler attatched and need a reload. I did not finded a way to "unatatche" the handler, but that is probaply not nececary anyway...
All other options are updated silently.

[Cube707]

I am not sure if the functionalaty should stay gloabal or if it should be put inside the activate() function?

[Josee9988]

Sorry for the delay, I don't know why but I didn't get a notification!

I've been testing out your code and it seems pretty good.

The functionality is good globally as you have made.

I would leave the second if statement only as if (event.affectsConfiguration('MinifyAll') so it triggers all MinifyAll changes.

Also, the handler is not useful in this case. It is making the configuration being loaded twice (when the user clicks reload and VSCode restarts, MinifyAll will load the configuration inside the handler and then it will load it again in the call export var settings: IUserSettings = getUserSettings();

So that variable should be left as a const as it was, and the callback could be safely removed!

---

Great job as always @Cube707 thanks for making these little improvements.

Modify these things and pull request it, or just pull request it and I'll modify it :D

[Josee9988]

Btw, when I mean the callback I want to say that unnecessary settings = getUserSettings(); call. As it is being called twice as I previously explained.

I would leave your code as:

// If the user changes the configurations, prompt for a window reload:
vscode.workspace.onDidChangeConfiguration((event) => {
    if (event.affectsConfiguration('MinifyAll')) {
        if (event.affectsConfiguration('MinifyAll')) {
            const reload = 'Reload';
            vscode.window.showInformationMessage(
                'Reload window in order for your changes in MinifyAll configuration to take effect.',
                reload, 'Cancel',
            ).then((selectedAction) => {
                if (selectedAction === reload) {
                    vscode.commands.executeCommand('workbench.action.reloadWindow');
                }
            });
        }
    }
});

Check it out and then you can pull-request it. :D

[Cube707]

What I was trying to do is this:
Whenever a "normal" option is changed, just call settings = getUserSettings(); to update the settings in the internal variable. This will happen silently without the user noticing.
The two options minifyOnSave and minifyOnSaveToNewFile are special, as they have a callback handler attacht to them, see here:

https://github.com/Josee9988/MinifyAll/blob/d5e671847a2456e7de3447faad1b2dd8f1341c01/src/main.ts#L54-L61

So a reload is nececary to run that code again. Thats why there are two seperate if statements.

Also after the "reload Windows" command is issued, the code after that will probaly not run and so nothing will be loaded twice. but I will add a return statement just to be sure.

[Josee9988]

LGTM. Try it and do some debugging and tell me

[Cube707]

I have it implementet and it should work now. I will do some more debugging and errorhunting tomorrow and open a pullrequest when I am sure I didn't do something to stupid.. XD

[Cube707]

I could also put a else statement to make it cleare that the code won't run if the second if triggers:

} else {
    // Update the settings:     
    settings = getUserSettings();
    terserMinifierOptions
}

[Josee9988]

2.10.0 is released. Check it out

Thanks again ❤️