settings: load settings-extra.json file

[matttbe]

This allows users to manage extra settings in an external file, e.g. to use one from another repository where the file can be tracked and even shared.

Also, by loading it at the end, it can override some "default" settings.

[bjackman]

Hey Matthieu thanks for this PR! I don't offer my opinion with much force because I don't actually use or contribute to this toolset very much, but I was made aware of this because I originally wrote the code the @FlorentRevest used for the JSonnit biz here so I have pondered this topic a bit.

I think there are two different questions at play here:

1. You wanna override something that the JSonnet sets, and you can't do that right now (right?). This is obviously a shortcoming of this tool and should definitely be fixed.

2. You wanna have a more flexible/reusable way to manage your VSCode configuration? This is a much wider-open problem that I think is kinda orthogonal to this repo and relevant for VSCode in general. And I think maybe jumping to a solution on that one might create annoying technical debt that makes this tool less usable in the long run.

   Like, maybe a full solution here would be to let the user write their own JSonnet and then they can arbitrarily override/inherit/modify the procided config. Or maybe it's something like just including and integrating with this extension. I haven't really researched it properly, not sure.

So yeah maybe it's worth first briefly discussing part 1 in isolation, if we can easily fix that then go ahead, and then discuss part 2 a bit more leisurely? What are the specific configs that need to be overridden? Do you think it would work to instead just make the JSonnet file a bit more advanced than just doing a + operation, e.g. for certain settings we might wanna check if the user already explicitly set them and in that case leave them in place.

Then again, I might just be over-thinking this...

[bjackman]

e.g. for certain settings we might wanna check if the user already explicitly set them and in that case leave them in place.

Oh. No, we can't do this, because we don't know what was set deliberately by the user and what was just set by a previous run of the JSonnet.

Hmm. It kinda makes me feel like we do just wanna let the user write JSonnet... but then we are asking people to learn a new language (even if it's not a very hard one...)

[matttbe]

Hey Brendan,

Thank you for your reply, and for having written the JSonnet biz!

You wanna override something that the JSonnet sets, and you can't do that right now (right?). This is obviously a shortcoming of this tool and should definitely be fixed.

That's not my first goal, and currently, I'm not doing that. I thought about that just before creating this pull-request: maybe some people will want to change some settings, e.g. disabling checkpatch, change the Git remotes, etc.

My first goal -- linked to PR #5 -- is to share settings that are specific to a development tool we use with the MPTCP subsystem. In short, all devs tools are "packaged" in a Docker container: compiler, QEmu, tools for tests, etc.. Recently, a new contributor was interested to use VSCode with our dev tool, so I checked what modifications were needed. It is easier to say "put all these files in this directory", than "change this setting to this value" or "modify this file to add this in the middle" (especially if there are more settings that need to be modified later), see these explanations.

In this context, no default settings are overridden, so we could change the order in the JSonnet file if you think it is better. I just liked the idea we could override default settings if needed :)

You wanna have a more flexible/reusable way to manage your VSCode configuration?

Not really. It is just clearer to put all my custom settings in a separate file :)
Personally, I don't need to modify the .jsonnet file, I don't need to do anything complex with it.

[matttbe]

Hello,

Is there anything I can do here? Would you prefer if this new extra JSonnet file don't override the default settings?

[bjackman]

Oh hey sorry I totally forgot that this was pending on me! Let me chat to @FlorentRevest tomorrow and we can figure out the way forward

[matttbe]

Hello,

@bjackman, by chance, did you manage to chat with @FlorentRevest about this PR? :)

[bjackman]

Ah - really sorry for causing a delay. I have spoken with Florent and I think I am fine with whatever - my concerns were pretty asbtract and aesthetic but it's more important now to get something that works.

[FlorentRevest]

Hey Matthieu!

Thanks for bearing with us and sorry it took us so long to get back to you... :') I was also caught up in some abstract design reflections but agree with Brendan we should just move on and land something that helps you out at this point.

---

I think our current JSonnet solution is a bit overengineered what it's trying to solve. It's nice because people can just edit their settings.json from the Settings UI and their settings will persist across updates but I don't really like maintaining this JSON merging strategy and was cautious before adding more complexity to it.

I was thinking we could maybe use something like https://marketplace.visualstudio.com/items?itemName=swellaby.workspace-config-plus instead. tl;dr: it's an extension that automatically maintains settings.json, tasks.json and launch.json files out of *.shared.json and *.local.json files. The idea would be to ship .shared.json files only, and that extension in extensions.json, and to let users create .local.json files if they want. Then, we can drop JSonnet entirely and have an easier update story (just overwriting settings.shared.json and no custom language).

The only drawback I can think of is that if users decide to use the settings UI, then the merged file will be modified instead of the .local.json file, which will probably be confusing. But I haven't tried to use the extension, maybe it shows a warning of some sort if people try to do that.

Does this seem more elegant ? If not, I'm also ok with merging the current PR as a first step, and if it bothers me enough, I'll eventually refactor it - eh. :)

---

If we meet at FOSDEM or LPC this year, the first beers are on me as a token of gratitude for your patience... :)

[matttbe]

Hey Florent!

Thanks for bearing with us and sorry it took us so long to get back to you... :') I was also caught up in some abstract design reflections but agree with Brendan we should just move on and land something that helps you out at this point.

No problem! :)

I think our current JSonnet solution is a bit overengineered what it's trying to solve. It's nice because people can just edit their settings.json from the Settings UI and their settings will persist across updates but I don't really like maintaining this JSON merging strategy and was cautious before adding more complexity to it.

I understand. But I also understand that the current version works, so not in a hurry to change ;)

I was thinking we could maybe use something like https://marketplace.visualstudio.com/items?itemName=swellaby.workspace-config-plus instead. tl;dr: it's an extension that automatically maintains settings.json, tasks.json and launch.json files out of *.shared.json and *.local.json files. The idea would be to ship .shared.json files only, and that extension in extensions.json, and to let users create .local.json files if they want. Then, we can drop JSonnet entirely and have an easier update story (just overwriting settings.shared.json and no custom language).

Sounds good. In my case, I wanted to have another extra JSON file, similar to the local*.sh files, so people can easily use this extension with a docker image I maintain. The docker image has all the tooling, can handle the build, start a VM (thanks to virtme-ng), running the tests, etc. so it simply uses it instead of the default wrapper.

The only drawback I can think of is that if users decide to use the settings UI, then the merged file will be modified instead of the .local.json file, which will probably be confusing. But I haven't tried to use the extension, maybe it shows a warning of some sort if people try to do that.

I don't know the extension, but I guess the people behind it are aware of this limitation.

Does this seem more elegant ? If not, I'm also ok with merging the current PR as a first step, and if it bothers me enough, I'll eventually refactor it - eh. :)

Yes, it is!
In the meantime, don't hesitate to already merge the current PR, so it is easier to use this extension for some users interested in MPTCP ;)

If we meet at FOSDEM or LPC this year, the first beers are on me as a token of gratitude for your patience... :)

:-D
See you at FOSDEM then ;-)