joshmedeski / sesh

Smart session manager for the terminal
MIT License
410 stars 16 forks source link

feat(config): include additional config files #84

Closed 0xRichardH closed 3 months ago

0xRichardH commented 3 months ago

Background

Usage

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
```toml
# ~/.config/sesh/local/sesh.toml
[[startup_scripts]]
session_path = "~/dev/third_session"
script_path = "~/.config/sesh/scripts/third_script"

Test Cases

0xRichardH commented 3 months ago

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

joshmedeski commented 3 months ago

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

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.

image
joshmedeski commented 3 months ago

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

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

joshmedeski commented 3 months ago

@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 commented 3 months ago

https://github.com/toml-lang/toml/issues/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 https://github.com/toml-lang/toml/issues/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 commented 3 months ago

Thanks! I'll look it over.