Skip to content

Re-thinking: Override values #327

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Tracked by #376
matthiasbeyer opened this issue Apr 23, 2022 · 7 comments
Open
Tracked by #376

Re-thinking: Override values #327

matthiasbeyer opened this issue Apr 23, 2022 · 7 comments

Comments

@matthiasbeyer
Copy link
Member

Right now we support overriding values.

Do we want to support these in the future? What are the usecases? How do they differ from default values?

@dnaka91
Copy link

dnaka91 commented May 1, 2022

Not just overring, but deleting values would be important, I believe.

Let's say we have several sources for a set of settings, but some of them should only come from one source. Then we could erase certain settings from lower priority layers, before we apply the higher ranked layers that are allowed to supply said settings.

@szarykott
Copy link
Contributor

szarykott commented May 8, 2022

@dnaka91 is there a real-life use case for it? Does current layering mechanism not support it? After all, values from subsequent layers override those from previous.

@matthiasbeyer As for overrides, I think they should stay, as they are not a big burden and some people might be using them (backwards compatibility that this crate, AFAIK has maintained since inception). And about "how do they differ from default ones" (I assume this is not rethorical question) - overrides are values that override any other source, regardless of the order, while defaults are "lowest level layer" - any other source can override them.

@matthiasbeyer matthiasbeyer mentioned this issue Sep 17, 2022
15 tasks
@martinvonz
Copy link
Contributor

FWIW, I've found overrides and default a bit confusing. We have defined out configs by just layering source. It seems like default values can be modeled as lower-level source and overrides can be modeled as a higher-level source. We do use overrides, but only in sources without other values. Here's how we do it: https://github.com/martinvonz/jj/blob/a2d2da4d483874258e0026d4583e7952ff21bafb/src/config.rs#L114-L147. That seems to have worked well so far. I had actually not even realized until now that there were default values too. I also had not reflected on what "override" really meant. Does an override in a lower-level layer override a config in a higher-level layer? I haven't checked explicitly but I think our tests would have failed if it worked that way.

@matthiasbeyer
Copy link
Member Author

It seems like default values can be modeled as lower-level source and overrides can be modeled as a higher-level source.

That's exactly the way I implemented it in the rewrite! 😆

@martinvonz
Copy link
Contributor

I had not heard about the rewrite before. What's the status? Will it be released as config 0.14.0?

@matthiasbeyer
Copy link
Member Author

The progress can be seen here: https://github.com/matthiasbeyer/config-rs-ng/ but it is not near anything finished yet. Also, I am not sure whether it will be 0.14.0 or something down the road.
It will be released once it is finished, but yes, if we decide that it is the right way to go, the codebase will be merged into config-rs and become config-rs!

@dnaka91
Copy link

dnaka91 commented Dec 21, 2022

@szarykott Sorry I totally forgot to reply to this.

The use case for deleting values would be, that we could restrict certain values to only be available on a specific layer. In Trunk, we have a few settings that should only be available to be set from the UI, as they're specific to each invocation and don't make much sense to be set from a config file.

Or sometimes where there can be a user-level config file, and we don't want to load certain settings from that file, as they're considered project specific. That is not currently the case, but might be something we do in the future.

Actually, maybe deleting wouldn't even be the right choice, but eventually something like filtering patterns instead (including globbing)?

@epage epage removed the re-think label Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants