Read bat config from multiple config files #691
Conversation
| @@ -16,8 +16,22 @@ pub fn config_file() -> PathBuf { | |||
| } | |||
|
|
|||
| pub fn get_args_from_config_file() -> Result<Vec<OsString>, shell_words::ParseError> { | |||
| let mut config_file_path = vec![PathBuf::from("/etc/bat/config")]; | |||
| config_file_path.push(default_config_path()); | |||
There was a problem hiding this comment.
so how does that work?
/etc/bat/config is always included but ~/.config/bat/config might be overwritten by BAT_CONFIG_PATH? Is that a good idea?
There was a problem hiding this comment.
@sharkdp Thanks for the review. I'm thinking of moving the logic for getting path from env var BAT_CONFIG_PATH to a separate function (something like get_config_file_path_from_env and default_config_path() will just return ~/.config/bat/config.
| let mut config_args = vec![]; | ||
|
|
||
| for path in config_file_path.iter() { | ||
| let args = read_args_from_config_file(path.to_path_buf()); |
There was a problem hiding this comment.
I think you could avoid the .to_path_buf() if read_args_from_config_file simply takes a &Path.
| for path in config_file_path.iter() { | ||
| let args = read_args_from_config_file(path.to_path_buf()); | ||
| config_args.append(&mut args.unwrap()); | ||
| } |
There was a problem hiding this comment.
It's not necessary, but the whole loop could probably be written in functional style with .map(…) and .collect(). This would also avoid the mut on config_args.
Something like (not tested and most probably wrong):
let config_args = config_file_path
.iter()
.map(read_args_from_config_file)
.transpose()?
.flatten()
.collect();
| @@ -16,8 +16,22 @@ pub fn config_file() -> PathBuf { | |||
| } | |||
|
|
|||
| pub fn get_args_from_config_file() -> Result<Vec<OsString>, shell_words::ParseError> { | |||
| let mut config_file_path = vec![PathBuf::from("/etc/bat/config")]; | |||
There was a problem hiding this comment.
config_file_path => config_file_paths?
|
Going to close this for now, as there is no further feedback. |
Summary
Fixes: #668