Skip to content
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.

Sign up for GitHub

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

fix: allow config sessions w/ duplicate paths #125

Closed
wants to merge 3 commits into from
Closed

fix: allow config sessions w/ duplicate paths #125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
09d5913
test: Not ban config sessions with duplicate paths
kristoferfannar Jun 12, 2024
543bb5c
Always display inactive config sessions
kristoferfannar Jun 26, 2024
8481667
test: Ensure parsed config paths are absolute
kristoferfannar Jun 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,23 @@ func (d *DefaultConfigDirectoryFetcher) GetUserConfigDir() (string, error) {
}
}

// Takes as input the user defined sessions from their config
// and cleans all paths to be absolute
func (c *Config) CleanPaths() error {
switch runtime.GOOS {
case "windows":
return nil

default:
for idx, sessionConfig := range c.SessionConfigs {
// convert user home dirs to absolute paths
c.SessionConfigs[idx].Path = dir.AlternatePath(sessionConfig.Path)
c.SessionConfigs[idx].StartupScript = dir.AlternatePath(sessionConfig.StartupScript)
}
Copy link
Contributor Author

@kristoferfannar kristoferfannar Jun 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed you're converting the sessionConfig paths to absolute paths lazily (at the moment its needed) in a couple of places.
For my Path+Name comparison of config sessions and active tmux sessions (in listConfigSessions()) I needed to convert to absolute again.

I figured that this might rather be done only once greedily (at parse time).

}
return nil
}

func parseConfigFromFile(configPath string, config *Config) error {
file, err := os.ReadFile(configPath)
if err != nil {
Expand All @@ -63,6 +80,13 @@ func parseConfigFromFile(configPath string, config *Config) error {
if err != nil {
return fmt.Errorf(": %s", err)
}

// cleanup potentially messy user paths
err = config.CleanPaths()
if err != nil {
return err
}

if len(config.ImportPaths) > 0 {
for _, path := range config.ImportPaths {
importConfig := Config{}
Expand Down
156 changes: 116 additions & 40 deletions config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,51 +19,76 @@ func (m *mockConfigDirectoryFetcher) GetUserConfigDir() (string, error) {
return m.dir, nil
}

func prepareSeshConfig(t *testing.T) string {
type MockSeshConfig struct {
Name string
Contents string
Imports string
}

func prepareSeshConfig(t *testing.T, mockSeshConfigs []MockSeshConfig) string {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inject a MockSeshConfig list to enable this function to be used multiple times for many tests.

This allows the caller to create their own MockSeshConfig and get a mock config back.

userConfigPath, err := os.MkdirTemp(os.TempDir(), "config")
if err != nil {
t.Fatal(err)
}
if err := os.MkdirAll(path.Join(userConfigPath, "sesh"), fs.ModePerm); err != nil {
t.Fatal(err)
}
tempConfigPath := path.Join(userConfigPath, "sesh", "sesh.toml")
secondTempConfigPath := path.Join(userConfigPath, "sesh", "sesh2.toml")

err = os.WriteFile(tempConfigPath, []byte(fmt.Sprintf(`
import = ["%s"]
[default_session]
startup_script = "default"

[[session]]
path = "~/dev/first_session"
startup_script = "~/.config/sesh/scripts/first_script"

[[session]]
path = "~/dev/second_session"
startup_script = "~/.config/sesh/scripts/second_script"
`, secondTempConfigPath),
), fs.ModePerm)
if err != nil {
t.Fatal(err)
}
// create a temp config file for each supplied config
for _, mockSesh := range mockSeshConfigs {
tempConfigPath := path.Join(userConfigPath, "sesh", mockSesh.Name)

err = os.WriteFile(secondTempConfigPath, []byte(`
[[session]]
path = "~/dev/third_session"
startup_script = "~/.config/sesh/scripts/third_script"
`), fs.ModePerm)
if err != nil {
t.Fatal(err)
var contents string
if mockSesh.Imports != "" {
contents = fmt.Sprintf("import = [\"%s\"]\n%s", path.Join(userConfigPath, "sesh", mockSesh.Imports), mockSesh.Contents)
} else {
contents = mockSesh.Contents
}
err = os.WriteFile(tempConfigPath, []byte(contents), fs.ModePerm)

if err != nil {
t.Fatal(err)
}
}

return userConfigPath
}

func TestParseConfigFile(t *testing.T) {
t.Parallel()
home, err := os.UserHomeDir()
if err != nil {
t.Fatal("unable to get user's home directory")
}

userConfigPath := prepareSeshConfig(t)
mockSessions := []MockSeshConfig{
{
Name: "sesh.toml",
Contents: fmt.Sprintf(`
[default_session]
startup_script = "default"

[[session]]
path = "~/dev/first_session"
startup_script = "~/.config/sesh/scripts/first_script"

[[session]]
path = "%s/dev/second_session"
startup_script = "%s/.config/sesh/scripts/second_script"
`, home, home),
Imports: "sesh2.toml",
},
{
Name: "sesh2.toml",
Contents: `
[[session]]
path = "~/dev/third_session"
startup_script = "~/.config/sesh/scripts/third_script"
`,
},
}

userConfigPath := prepareSeshConfig(t, mockSessions)
defer os.Remove(userConfigPath)

t.Run("ParseConfigFile", func(t *testing.T) {
Expand All @@ -84,23 +109,23 @@ func TestParseConfigFile(t *testing.T) {
if len(config.SessionConfigs) != 3 {
t.Errorf("Expected %d, got %d", 3, len(config.SessionConfigs))
}
if config.SessionConfigs[0].Path != "~/dev/first_session" {
t.Errorf("Expected %s, got %s", "~/dev/first_session", config.SessionConfigs[0].Path)
if config.SessionConfigs[0].Path != (home + "/dev/first_session") {
t.Errorf("Expected %s, got %s", home+"/dev/first_session", config.SessionConfigs[0].Path)
}
if config.SessionConfigs[0].StartupScript != "~/.config/sesh/scripts/first_script" {
t.Errorf("Expected %s, got %s", "~/.config/sesh/scripts/first_script", config.SessionConfigs[0].StartupScript)
if config.SessionConfigs[0].StartupScript != (home + "/.config/sesh/scripts/first_script") {
t.Errorf("Expected %s, got %s", home+"/.config/sesh/scripts/first_script", config.SessionConfigs[0].StartupScript)
}
if config.SessionConfigs[1].Path != "~/dev/second_session" {
t.Errorf("Expected %s, got %s", "~/dev/second_session", config.SessionConfigs[1].Path)
if config.SessionConfigs[1].Path != (home + "/dev/second_session") {
t.Errorf("Expected %s, got %s", home+"/dev/second_session", config.SessionConfigs[1].Path)
}
if config.SessionConfigs[1].StartupScript != "~/.config/sesh/scripts/second_script" {
t.Errorf("Expected %s, got %s", "~/.config/sesh/scripts/second_script", config.SessionConfigs[1].StartupScript)
if config.SessionConfigs[1].StartupScript != (home + "/.config/sesh/scripts/second_script") {
t.Errorf("Expected %s, got %s", home+"/.config/sesh/scripts/second_script", config.SessionConfigs[1].StartupScript)
}
if config.SessionConfigs[2].Path != "~/dev/third_session" {
t.Errorf("Expected %s, got %s", "~/dev/third_session", config.SessionConfigs[2].Path)
if config.SessionConfigs[2].Path != (home + "/dev/third_session") {
t.Errorf("Expected %s, got %s", home+"/dev/third_session", config.SessionConfigs[2].Path)
}
if config.SessionConfigs[2].StartupScript != "~/.config/sesh/scripts/third_script" {
t.Errorf("Expected %s, got %s", "~/.config/sesh/scripts/third_script", config.SessionConfigs[2].StartupScript)
if config.SessionConfigs[2].StartupScript != (home + "/.config/sesh/scripts/third_script") {
t.Errorf("Expected %s, got %s", home+"/.config/sesh/scripts/third_script", config.SessionConfigs[2].StartupScript)
}
})
}
Expand Down Expand Up @@ -182,3 +207,54 @@ func BenchmarkParseConfigFile(b *testing.B) {
})
}
}

func TestConfigPathsAreCleaned(t *testing.T) {
t.Parallel()

home, err := os.UserHomeDir()
if err != nil {
t.Fatalf("failed to get home dir: %s", err.Error())
}

mockSessions := []MockSeshConfig{
{
Name: "sesh.toml",
Contents: fmt.Sprintf(`
[default_session]
startup_script = "default"

[[session]]
path = "~/tilde-prefixed"
name = "my-tilde-prefixed-session"
startup_script = "~/my_script"

[[session]]
path = "%s/tilde-prefixed"
name = "my-tilde-prefixed-session"
startup_script = "%s/my_script"
`, home, home),
},
}

userConfigPath := prepareSeshConfig(t, mockSessions)
defer os.Remove(userConfigPath)

fetcher := &mockConfigDirectoryFetcher{dir: userConfigPath}
cfg := config.ParseConfigFile(fetcher)

err = cfg.CleanPaths()
if err != nil {
t.Fatalf("failed to clean paths: %s", err.Error())
}

// test all config sessions
for _, sessionConfig := range cfg.SessionConfigs {
if !strings.HasPrefix(sessionConfig.Path, home) {
t.Fatalf("config session path is not absolute")
}

if !strings.HasPrefix(sessionConfig.StartupScript, home) {
t.Fatalf("config session startup script is not absolute")
}
}
}
3 changes: 1 addition & 2 deletions session/determine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"log"

"github.com/joshmedeski/sesh/config"
"github.com/joshmedeski/sesh/dir"
"github.com/joshmedeski/sesh/name"
)

Expand All @@ -16,7 +15,7 @@ func isConfigSession(choice string) *Session {
return &Session{
Src: "config",
Name: sessionConfig.Name,
Path: dir.AlternatePath(sessionConfig.Path),
Path: sessionConfig.Path,
}
}
}
Expand Down
25 changes: 15 additions & 10 deletions session/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"reflect"

"github.com/joshmedeski/sesh/config"
"github.com/joshmedeski/sesh/dir"
"github.com/joshmedeski/sesh/tmux"
"github.com/joshmedeski/sesh/zoxide"
)
Expand All @@ -27,16 +26,19 @@ func checkAnyTrue(s interface{}) bool {
return false
}

func makeSessionsMap(sessions []Session) map[string]bool {
// Takes as input a list of sessions and an injected mapper function
// Returns a map using the mapper function
// to show existing sessions
func makeSessionsMap(sessions []Session, mapper func(s Session) string) map[string]bool {
sessionMap := make(map[string]bool, len(sessions))
for _, session := range sessions {
sessionMap[session.Path] = true
sessionMap[mapper(session)] = true
}
return sessionMap
}

func isInSessionMap(sessionMap map[string]bool, path string) bool {
_, exists := sessionMap[path]
func isInSessionMap(sessionMap map[string]bool, key string) bool {
_, exists := sessionMap[key]
return exists
}

Expand All @@ -62,14 +64,17 @@ func listTmuxSessions(o Options) (sessions []Session, err error) {

func listConfigSessions(c *config.Config, existingSessions []Session) (sessions []Session, err error) {
var configSessions []Session
sessionMap := makeSessionsMap(existingSessions)
fString := "%s-%s"

// filter sessions by name+path combination
sessionMap := makeSessionsMap(existingSessions, func(s Session) string { return fmt.Sprintf(fString, s.Name, s.Path) })

for _, sessionConfig := range c.SessionConfigs {
path := dir.AlternatePath(sessionConfig.Path)
if !isInSessionMap(sessionMap, path) && sessionConfig.Name != "" {
if !isInSessionMap(sessionMap, fmt.Sprintf(fString, sessionConfig.Name, sessionConfig.Path)) && sessionConfig.Name != "" {
configSessions = append(configSessions, Session{
Src: "config",
Name: sessionConfig.Name,
Path: path,
Path: sessionConfig.Path,
})
}
}
Expand All @@ -82,7 +87,7 @@ func listZoxideSessions(existingSessions []Session) (sessions []Session, err err
return nil, fmt.Errorf("couldn't list zoxide results: %q", err)
}
var zoxideSessions []Session
sessionMap := makeSessionsMap(existingSessions)
sessionMap := makeSessionsMap(existingSessions, func(s Session) string { return s.Path })
for _, result := range results {
if !isInSessionMap(sessionMap, result.Path) {
zoxideSessions = append(zoxideSessions, Session{
Expand Down
Loading