-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
feat: support override in config file #1565
base: Alpha
Are you sure you want to change the base?
feat: support override in config file #1565
Conversation
@Benyamin-Tehrani Thanks, but you need run |
ece423e
to
d3ed64a
Compare
Thanks, I forgot that. Force-pushed |
According to the mergo documentation, zero values will not be overwritten. This will lead to a defect that a string cannot be overwritten to empty, and an int cannot be overwritten to 0. It may be difficult for users to understand and accept this behavior? |
Sure, I have thought about this problem. Maybe some workaround like BTW, it's not easy to find out an scenario that need to overwrite an existing value with 0 or ""? |
At the same time, golang will also treat false as zero value. These restrictions will greatly increase the mental burden of users when using it. For example, in certain circumstances, set allow-lan to false, or in certain circumstances, set external-controller to empty to turn off this function. The above are just two examples to prove that such scenarios are not difficult to find. |
Oh, I have found another params of mergo about EmptyValue overwrite policy. I'll look into it. |
This is a bit of a tricky one. In essence the problem is that golang doesn't use nil as a common null representation, while string & numbers still use zero as their nil. So it's hard for libraries to distinguish these types of nulls from zeroes. |
This is why hub/route/configs.go repeatedly defines a large number of configuration types and changes the attributes to pointers to overwrite them one by one. |
Perhaps patching the original yaml directly can avoid falling into golang's zero value problem, but it should be noted that the existing order needs to be kept unchanged when serializing and deserializing the map in yaml. |
I can also do this for This should be the easiest way, I think, values that don't exist in yaml will be set to nil pointers. Then just merge with mergo normally, and those not modified will be ignored as they're nil pointers. Is it right? I have not read the code in depth. Maybe patch in |
In fact, the value of RawConfig is widely used in other GUI client projects. At the same time, the DefaultRawConfig function defines many special initial values for RawConfig. The maintenance cost of significantly modifying its definition is unacceptable. |
Finally figured out a way to solve this problem. Idea:Process config override in
All tests are passed. Modified go-yaml:I have forked The repo is https://github.com/Benyamin-Tehrani/yaml. Maybe it's better to move this repo into MetacubeX for easier maintenance in the future. Considering that the original All tests in this |
Sorry, I misunderstood the purpose of this issue. |
As written in the example, It eliminates the need to save the config file on each device individually, and meanwhile ease the synchronization of the configs across multiple devices. In addition, this feature gives support to configuration overriding for all clients, no longer need to set override settings seperately on all devices :-P |
Any process on this PR? ;-) @wwqgtxx |
This implementation looks good, but I think we can refer to the syntax of mihomo-party and simplify the |
The mihomo party's design is indeed clearer, but it would require a complete reimplementation of the deep merge logic I think ;-) . It looks like the "merge option" may appear at any depth layer of the yaml config? |
This is indeed the case. This should not be a big problem for modifying the YAML library. The deep deserialization is done by recursive calls. |
@wwqgtxx I suddenly realized that the config with key begins with
|
22addc6
to
5772507
Compare
Perhaps we can replace ^nameserver-policy:
+.google.cn:
- 8.8.8.8#Proxy
- 1.1.1.1#Proxy and nameserver-policy^:
+.google.cn:
- 8.8.8.8#Proxy
- 1.1.1.1#Proxy |
I'd like to share some ideas for your consideration (Syntax fix only for keys starting with nameserver-policy:
# override
<+.google.cn>:
- 8.8.8.8#Proxy
- 1.1.1.1#Proxy
# prepend
+<+.google.cn>:
- 8.8.8.8#Proxy
- 1.1.1.1#Proxy
# append
<+.google.cn>+:
- 8.8.8.8#Proxy
- 1.1.1.1#Proxy |
Signed-off-by: Benyamin-Tehrani <[email protected]>
634c806
to
9470f3a
Compare
Intro
Support
override
section in config file. It contains the configurations that will be merged into the original one, and determine whether to merge or not based on the given conditions.Solved issue #1476
Format
Available Conditions
Example