Restructure and rename settings

[oidro]

This pull request:

• Renames settings to better fit with VSCode's UI-based settings display (it expects camelCase setting names, the extension formerly used snake_case)

• Groups settings into categories

• Slightly rewords some setting descriptions (see below)

  Setting (new name)	Before	After
  godot-tools.lsp.serverProtocol	You have restart VSCode editor after change this value.	You must restart VSCode after changing this value.
  godot-tools.lsp.autoReconnect.enabled	Whether the plugin should attempt to reconnect	Whether the plugin should attempt to reconnect to the GDScript language server
  godot-tools.lsp.serverProtocol (option descriptions)	Using WebSocket protocol to connect to Godot 3.2 and Godot 3.2.1	Use the WebSocket protocol to connect to Godot 3.2 and Godot 3.2.1

• Fixes a bug with utils.get_configuration that would result in the lsp.autoReconnect.enabled (formerly reconnect_automatically) setting being completely ignored

Before:

After:

(Renaming "Godot Tools configuration" to "Godot Tools" complies with Microsoft's official recommendation for settings category names)

[oidro]

Do note that this will almost certainly invalidate users' previous settings, something that (if this PR is accepted) should be mentioned in the changelog. I can make a table mapping every old setting to its new name, if desired. Here is that table:

Old Name	New Name
godot_tools.gdscript_lsp_server_protocol	godot-tools.lsp.serverProtocol
godot_tools.gdscript_lsp_server_host	godot-tools.lsp.serverHost
godot_tools.gdscript_lsp_server_port	godot-tools.lsp.serverPort
godot_tools.editor_path	godot-tools.editorPath
godot_tools.scene_file_config	godot-tools.sceneFileConfig
godot_tools.reconnect_automatically	godot-tools.lsp.autoReconnect.enabled
godot_tools.reconnect_cooldown	godot-tools.lsp.autoReconnect.cooldown
godot_tools.reconnect_attempts	godot-tools.lsp.autoReconnect.attempts
godot_tools.force_visible_collision_shapes	godot-tools.forceVisibleCollisionShapes
godot_tools.force_visible_nav_mesh	godot-tools.forceVisibleNavMesh

[Calinou]

Since this is a compatibility-breaking change, the next version of this extension will be 2.0.0.

[Calinou]

PS: Do you know how to make a toast pop up after the user updates the extension? This would be useful to show a message about the renamed settings.

[oidro]

PS: Do you know how to make a toast pop up after the user updates the extension? This would be useful to show a message about the renamed settings.

Unfortunately there's no official API for this yet--however, the GitLens extension implements such a feature, which I took inspiration from to make f9ed7f6. Hopefully that's good!

If the updated setting names are going to be in the changelog, I can update the message to say something like "Please view the changelog to see the updated settings." (I would add a button to open the changelog, but unfortunately there doesn't seem to be a standard way to make a popup button open a web browser, and links inside of popups aren't automatically highlighted, so including a URL wouldn't work very well either.)

[DaelonSuzuka]

Do note that this will almost certainly invalidate users' previous settings

Since this is a compatibility-breaking change, the next version of this extension will be 2.0.0.

Are we sure about this?

The old settings will still be in the user's settings.json, why can't we just manually read the old style settings, copy their values to the new settings, and remove the old ones? There's only a dozen settings, the code should be trivial. I would bet most users don't change even one setting, so I'm not sure how they'd notice.

We can still throw a popup saying that the user's settings have been converted to the new style names, and maybe tell them to double check their extension settings.

Since the number of old-style settings will never increase, there also shouldn't be any (much) maintenance cost to just leave the converter routine in place

[oidro]

The old settings will still be in the user's settings.json, why can't we just manually read the old style settings, copy their values to the new settings, and remove the old ones? There's only a dozen settings, the code should be trivial. I would bet most users don't change even one setting, so I'm not sure how they'd notice.

You're right, we absolutely could do that. I just pushed 8f307ae implementing it.

Based on the milestone this PR was assigned (1.5.0), I changed the update text to:

Settings from godot-tools version <1.5.0 have been updated to the new format. Please view the changelog for version 1.5.0 for more information.

That message displays conditionally, only if any settings have actually been converted.

[DaelonSuzuka]

That's awesome, thank you for updating this. I'll try to get it reviewed this week.

Based on the milestone this PR was assigned (1.5.0), I changed the update text to:

The milestone wasn't a hard decision, mostly I wanted to indicate that I was serious about getting this merged, but not trying to force it into the next release.

That message displays conditionally, only if any settings have actually been converted.

Sounds perfect to me, most users will never even need to know that we did this.

[DaelonSuzuka]

Sorry for the delay in reviewing: I've been under the weather lately.

I'm really excited for this PR. The new names look WAY better in the settings menu.

The settings converter worked great, once I remembered that the extension only activates when you touch a godot file or something. My extension host window didn't have a relevant file open, so it didn't trigger the extension to start and it left me very confused about why my setting hadn't been converted.

Update Message

I didn't see the update message the first time around and had to trick the extension into displaying it for me. It looks like this, all on one line with most of the message cut off:

Adding a button to the message caused the entire text to be shown, and making the user dismiss it might make it less missable:

The Okay button doesn't actually do anything.

Old Settings

While I was playing with this I had the thought that if a user jumps between different versions, then removing the old settings might cause an undesirable situation where they install 1.5.0, then install an older version and get confusing behavior. What do we think about creating new settings but leaving the old ones and removing them in a future version? In fact, plenty of other extensions just leave old stuff in settings.json forever, including Microsoft owned ones like the python one.

Side Note

Reading your code taught me about ExtensionContext.globalState (and workspaceState, storagePath, and globalStoragePath), so thanks a ton for that!

[oidro]

Thank you for the review! Apologies for my delay in responding.

---

What do we think about creating new settings but leaving the old ones and removing them in a future version?

I think that leaving the old ones is a good choice. As you mentioned, it goes along with the precedent set by other extensions, including Microsoft's. I'll update the converter routine to not delete anything.

As for removing them in the future, I believe it'd be best to just leave them there forever, as it avoids any strange conflicts if users (for whatever reason) jump between various versions of the extension. Deleting stuff from the user's configuration did make me feel a bit uneasy anyways.

---

I didn't see the update message the first time around and had to trick the extension into displaying it for me.

I was worried about this, I hope it didn't just convert things silently. The logic for displaying the message will change slightly when I remove the deletion aspect of the converter routine, only displaying if there's an old-style configuration key without a matching new-style one (which it will add beforehand).

---

I'll get to work on the changes shortly, I'm a little busy due to the holidays but have more than enough time within the next few days.

(Also, thanks for noticing the message getting cut-off and adding the button!)

[DaelonSuzuka]

I believe it'd be best to just leave them there forever... Deleting stuff from the user's configuration did make me feel a bit uneasy anyways.

I'm fully on board with that now. I hadn't really thought it through when I originally suggested deleting the original settings.

I was worried about this, I hope it didn't just convert things silently.

I'm fairly confident that I just missed the popup the first time around. Once I started trying to intentionally trigger the conversion, it happened every time I expected it.

[DaelonSuzuka]

I realized that half of the contributed items(commands/configurations) were still using a random assortment of kebab-case and snake_case, plus an uncomfortable mix of godot-tool and godot-tools. They're now all standardized on the camelCase godotTools.system.doSomething.

Only the string names of contributed items were changed, so there shouldn't be any behavior changes to the extension.

Before:

After:

[DaelonSuzuka]

@oidro Thank you so much for putting this PR together, this is a much-needed improvement.

@Calinou This is ready to merge whenever.

[atirut-w]

using a random assortment of kebab-case and snake_case, plus an uncomfortable mix of godot-tool and godot-tools

How did that even happen?

[DaelonSuzuka]

I have several planned features that are blocked by this PR, so I'm going to merge it even though I contributed to it.

Sorry in advance, @Calinou.

[oidro]

@oidro Thank you so much for putting this PR together, this is a much-needed improvement.

No problem, thanks for finishing it! Got tied up with work and totally forgot to myself, haha.

Slight word of caution, in its current state the converter still deletes old-style settings, but I understand if that's not a priority right now! Should just be a simple change in the logic of updateOldStyleSettings if anyone wants to take it up, though.

In any case, glad to see it merged! 🎉

[DaelonSuzuka]

its current state the converter still deletes old-style settings

Thanks for mentioning that, I definitely remembered reviewing a version with that already removed. I must have done it myself when I was messing around and then thrown it away.

I already have the fix in my next PR, alongside a handful of typos/missed configuration names.