-
Notifications
You must be signed in to change notification settings - Fork 17
Create a YAML based configuration to customize key binding #14
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.
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
base: main
Are you sure you want to change the base?
Conversation
…onfig/RectangleWin/config.yaml if missing, instad of the current folder
…aveat of this is that there's no simple way to reload after changing and saving it in notepad.exe, due to the need for a more complex synchronization for this reload
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mostly good, I'll also give a try myself hands-on sometime.
configDir := filepath.Join(homeDir, DEFAULT_CONF_PATH_PREFIX) | ||
err := os.MkdirAll(configDir, 0755) | ||
if err != nil { | ||
fmt.Printf("Error creating directory under user's home folder: %s", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return the err actually, we wouldn't wanna ignore a failure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
conf.go
Outdated
// read or write the conf in current folder. | ||
return DEFAULT_CONF_NAME | ||
} | ||
configDir := filepath.Join(homeDir, DEFAULT_CONF_PATH_PREFIX) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
configDir := filepath.Join(homeDir, DEFAULT_CONF_PATH_PREFIX) | |
configDir := filepath.Join(homeDir, filepath.FromSlash(DEFAULT_CONF_PATH_PREFIX)) |
isn't this necessary to make /
work on win32?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done. (somehow it works on my machine still without it...) thanks for catching.
return configPath | ||
} | ||
|
||
func maybeDropExampleConfigFile(target string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO we should just fail/crash. you wanna do things like "ignore error and do this instead" in 1 place in the code, as close to the top of the main() as possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you prefer to move the calling places to main.go, please?
conf.go
Outdated
import ( | ||
"github.com/davecgh/go-spew/spew" | ||
"github.com/golobby/config/v3" | ||
"github.com/golobby/config/v3/pkg/feeder" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bit worried about security supply chain bringing in deps like this. could we avoid this and just stick with gopkg.in/yaml.v3 or something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done. thanks for the suggestion.
(there are some online comments/repo README suggesting that gopkg.in/yaml.v3 may emit worse error message when a syntax error is encountered though...)
config.example.yaml
Outdated
@@ -0,0 +1,70 @@ | |||
# This is a sample config file for RectangleWin. | |||
# See conf.go for details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ideally readme.md should have enough info
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated and removed.
conf.go
Outdated
fmt.Printf("Error creating directory under user's home folder: %s", err) | ||
// read or write the conf in current folder | ||
return DEFAULT_CONF_NAME | ||
return DEFAULT_CONF_NAME, errors.New("Failed to create folders under user's home directory") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the idiomatic error format for Go
return DEFAULT_CONF_NAME, errors.New("Failed to create folders under user's home directory") | |
return DEFAULT_CONF_NAME, fmt.Error("failed to create config folder in home dir: %w") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done. thanks!
…ad of golobby/config; also remove spew which is for debugging purposes only; also use fmt.Errorf(...) for errors
Create a YAML based configuration to customize key binding.
The YAML parsing logic is defined in conf.go. Long story short, I use golobby/config to parse them.
config.yaml is the config file name, with the recommended config.
A small update is added to main.go:
=> "fixconsole.FixConsoleIfNeeded()"
This fixes the binary to properly attach to standard input output on windows platform, to facilitate debugging.
If you have a strong opinion, I can remove spew and fixconsole from the deps to make it more lightweight in dependencies. However I found the outputs very helpful for debugging.