feat(config): include additional config files

[0xRichardH]

Background

• I will add the sesh config file to my GitHub repository dotfiles, but I don't want to commit some sensitive information to the public GitHub repository. For example:
  • I have the following sesh startup_scripts config, but I don't want to add the sensitive info (company name) ~/code/my_compay to my Github repository

    [[startup_scripts]]
    session_path = "~/code/my_compay/third_session"
    script_path = "~/.config/sesh/scripts/third_script"

• The solution is to allow the sesh config to include additional config files, and then I could exclude the additional config files from the git commit history.

Usage

• The custom sesh config

# ~/.config/sesh/sesh.toml

default_startup_script = "default"

[[startup_scripts]]
session_path = "~/dev/first_session"
script_path = "~/.config/sesh/scripts/first_script"

[[startup_scripts]]
session_path = "~/dev/second_session"
script_path = "~/.config/sesh/scripts/second_script"

[[extended_configs]]
path = "~/.config/sesh/local/sesh.toml"

• The following config is an additional config that will be included in the custom sesh config

# ~/.config/sesh/local/sesh.toml
[[startup_scripts]]
session_path = "~/dev/third_session"
script_path = "~/.config/sesh/scripts/third_script"

Test Cases

• All of the test cases are passed.

[joshmedeski]

I think this is an interesting idea and worth merging, I left some minor feedback on naming and performance.

[joshmedeski]

Please add a benchmark test and figure if there's a significant drop in performance if someone uses this feature a lot. I'd like to warn people in the README if heavy use of this feature causes performance problems.

[0xRichardH]

It makes sense. I will write some benchmark tests.

[0xRichardH]

go test -bench=. ./config/config_test.go -benchtime=10s -benchmem
goos: darwin
goarch: arm64
BenchmarkParseConfigFile/ParseConfigFile_1-8              514300             23670 ns/op            5176 B/op         52 allocs/op
BenchmarkParseConfigFile/ParseConfigFile_10-8              98485            122762 ns/op           27536 B/op        256 allocs/op
BenchmarkParseConfigFile/ParseConfigFile_100-8             10000           1171631 ns/op          249921 B/op       2242 allocs/op
BenchmarkParseConfigFile/ParseConfigFile_1000-8              910          12548132 ns/op         2515669 B/op      22050 allocs/op
BenchmarkParseConfigFile/ParseConfigFile_10000-8              73         164291629 ns/op        25620936 B/op     220066 allocs/op
PASS
ok      command-line-arguments  83.454s

[0xRichardH]

it affects the performance if there are a thousand separate extended config files. But I think the user only needs one extended_config in most cases. So I don't think there will be any performance issues.

[0xRichardH]

Hi @joshmedeski Do you know why the bench tests keep failing? Do you think it is a GitHub action resource issue?

[joshmedeski]

It might be timing out due to the long test.

Now that you've tested where the dropoff point is, let's remove the bigger performance tests so the testing can be sped up.

Maybe we just need to test that two works.

[0xRichardH]

I cannot figure out why the beach tests still fail on CI. The bench tests will fail even if I set the test input to 0 (which means not loading the extended/additional configs). So I disabled running bench tests for ParseConfigFile for now.

• btw this is my test branch to adjust the test input. Feat/config include additional config files test 0xRichardH/sesh#2

[joshmedeski]

It appears to be working now. Thanks for creating the tests, I mostly just wanted some awareness for where this feature drops off in performance, and we now have that answer.

I'll look over the code again and do some manual testing then I can merge and ship it, thanks!

[joshmedeski]

Looks good to me 👍

[joshmedeski]

I've got some more work to do this week, so I probably will do a release on Friday. Nice work!

[joshmedeski]

@0xRichardH did you play around with the built-in import feature from toml?

import = ["~/code/my_compay/sesh.toml"]

It didn't work on my first try, but this seems more simple and more inline with how toml works rather than us trying to create a brand new pattern.

We could even convert what we did to an array instead of grouping by [[extended_configs]].

What do you think? I want this to be as simple as possible before documenting and shipping it.

[0xRichardH]

toml-lang/toml#397

@0xRichardH did you play around with the built-in import feature from toml?

import = ["~/code/my_compay/sesh.toml"]

It didn't work on my first try, but this seems more simple and more inline with how toml works rather than us trying to create a brand new pattern.

We could even convert what we did to an array instead of grouping by [[extended_configs]].

What do you think? I want this to be as simple as possible before documenting and shipping it.

The toml itself doesn't support import feature toml-lang/toml#397 . but it will be more elegant to use import to replace extended_configs. I will create a new PR to make a change.

import = ["~/code/my_compay/sesh.toml"]

[joshmedeski]

Thanks! I'll look it over.