FlorentRevest / linux-kernel-vscode

Reference setup for Linux kernel development in VSCode
GNU General Public License v2.0
185 stars 15 forks source link

settings: load settings-extra.json file #6

Open matttbe opened 4 months ago

matttbe commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 2 months ago

Hello,

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

bjackman commented 1 month ago

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