zeroc-ice / vscode-slice

Slice syntax highlighter for Visual Studio Code
BSD 3-Clause "New" or "Revised" License
5 stars 4 forks source link

Make built-in References an Option #24

Closed InsertCreativityHere closed 7 months ago

InsertCreativityHere commented 7 months ago

Currently, when the server starts up, we create a default SliceConfig. This means that built_in_slice_path gets a default value of "".

If for some reason, this property isn't set yet, this will cause the extensions to search /slice/ for files, which will break.

Instead, it's safer to use an Option and only add it if it's set. And then it has a default value of None. Note, that this is what we do for the other paths in SliceConfig.


Also, it simplifies some logic under the assumption that root_uri is absolute. Guess I forgot to add this to my other PR, or maybe it got lost in the merge.

externl commented 7 months ago

As it stands right now I'm not sure this is really necessary. The slice dir can never be empty. Nothing is ever called on the language server until it's initialized, at which point the slice dir is set. I think it would make the most sense to also add the ability to enable/disable inclusion of the builtin slice files, but it should be a boolean on the config struct.

InsertCreativityHere commented 7 months ago

I opened this thinking we wouldn't be adding ConfigurationSet support for a while. Now that we are, this isn't necessary anymore, so I'm just going to close it, instead of risking the introduction of merge conflicts.