Implement config parsing #22
Conversation
cmds/choose.go
Outdated
| @@ -65,7 +66,8 @@ func Choose() *cli.Command { | |||
| } | |||
| choice := strings.TrimSpace(cmdOutput.String()) | |||
| // TODO: get choice from Session structs array | |||
| connect.Connect(choice, false, "") | |||
| config := config.ParseConfigFile() | |||
| connect.Connect(choice, false, "", &config) | |||
There was a problem hiding this comment.
Do you like passing all of the config into these functions? I'm not sure how to break up the config file yet but we may end up passing more data than is needed for each function.
There was a problem hiding this comment.
I think there are two approaches here: pass exactly what we need, or pass a reference to the config.Config struct. That way, we have all the data that is needed (and then some), but we aren't needlessly creating copies of the config struct. I'm open to either, but I will say that it's maybe a little hard to know at this point what we'll need to pass through for future config values and the parameters could get a bit out of hand if they're not all in a single object that you can pass through.
|
I'm checking with Tom about how he uses this parent dir feature. I'm gonna put this PR on hold until I decide we have something work using for a configuration file. |
Sounds good. Something that I think it could be used for is specifying startup scripts. I'm thinking it could be set up such that you link a path to a startup script wherever you want to store them, and it'll execute that script after creating the session. All good either way, happy to work on something else entirely in the meantime! |
|
For #7 |
|
@joshmedeski I implemented the startup scripts feature and added some docs to go along with it. For now, users can only run persistent scripts, i.e. the created session won't exit when the script exits. I think this is likely what people will want most of the time anyway, but we can implement the opposite behavior down the road if people are interested. Might need some help with resolving the merge conflicts, though... not entirely sure what the best way of doing that is with go.mod and go.sum. |
|
Another merge conflict. But for now can we drop the parent session name? I'd like to address #33 and I'll be adding nerd font support soon, which will be need a config option ;) |
|
Sorry for all the merge conflicts, @joebonneau! I'll review all of this tonight and hopefully we can get it merged very soon. |
| if err != nil { | ||
| return fmt.Errorf("unable to connect to %q: %w", choice, err) | ||
| } | ||
|
|
||
| if err := zoxide.Add(session.Path); err != nil { | ||
| if err = zoxide.Add(session.Path); err != nil { |
There was a problem hiding this comment.
| if err = zoxide.Add(session.Path); err != nil { | |
| if err := zoxide.Add(session.Path); err != nil { |
We need a : here please
Open discussion/items:
Here's what the config looks like so far (super barebones):
Also, not sure why my commits from the previous PR are showing up here. If there's some better way to be doing this, let me know... I basically just updated my fork from your repo and then added commits to it.