kcl-lang / kcl-go

KCL Go SDK
https://pkg.go.dev/kcl-lang.io/kcl-go@main
Apache License 2.0
52 stars 26 forks source link

WithSettings should merge options instead of overwriting #205

Closed FLAGLORD closed 10 months ago

FLAGLORD commented 10 months ago

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

kcl --help 的输出的信息中提到 -Y 应该接受一个列表

image

但是 sdk 中,kcl.WithSettings 的逻辑是覆盖,并不是合并,两者逻辑不一致

2. What did you expect to see? (Required)

3. What did you see instead (Required)

4. What is your KCL components version? (Required)

kclvm version is 0.4.4; checksum: c5339e572207211e46477825e8aca903

Peefy commented 10 months ago

Hi @FLAGLORD thank you for your feedback. I've checked the option merge logic in the latest version, maybe it's right. kcl.WithSettings returns a Option. The merge logic of Option is here: https://github.com/kcl-lang/kcl-go/blob/main/pkg/kcl/opt.go#L218 What is your opinion about this function?

FLAGLORD commented 10 months ago

Let's take a look at a example. Assuming that we have two setting files kcl_options.yaml and kcl_cli_configs.yaml. The content of kcl_options.yaml is like:

kcl_options:
    - key: xxxk
      value: xxxv

The content of kcl_cli_configs.yaml is like:

kcl_cli_configs:
    file:
        - main.k
        - xxx.k

I can use both of them like kcl -Y kcl_cli_configs.yaml kcl_options.yaml.

However in the golang-sdk, as WithSettings accepts only a string as its parameter, I invoke it like below:

kcl.Run("test.k", kcl.WithSettings("kcl_cli_configs.yaml"), kcl.WithSettings("kcl_options.yaml"))

It seems that the former option is overwritten by the later.

Peefy commented 10 months ago

@FLAGLORD

The right form is as follows

kcl.Run("test.k", kcl.WithSettings("kcl_cli_configs.yaml"), kcl.WithSettings("kcl_options.yaml"))

Besides, let me to give you a kusion example: https://github.com/KusionStack/kusion/blob/v0.7.4/pkg/generator/kcl/kcl_generator.go#L136

FLAGLORD commented 10 months ago

Thanks for your reply. Maybe I misunderstood the error info. I have followed the example above, it now tells me the plugin package is not found:

image

Did i miss something?

Peefy commented 10 months ago

Thanks for your reply. Maybe I misunderstood the error info. I have followed the example above, it now tells me the plugin package is not found:

image

Did i miss something?

For the internal plugin module, pls contact me on the dingtalk.