-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add support to save file with no extension #813
Conversation
The suppport introduced for files with no file extension is only partial as trying to save the config file would fail with `<file name> equires valid extensio` This adds support to saving such files
@sagikazarmark any chance to give this a look? it would be great to fully support config files with no extensions |
err := v.ReadConfig(bytes.NewBuffer(yamlExample)) | ||
if err != nil { | ||
t.Fatal(err) | ||
testCases := map[string]struct { |
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.
Nice refactor!
Currently, the config type is always inferred from the file name, not the original config type. So if the config type was yaml, but I provided a json file name, the output content was in JSON.
Can you please add a test case that verifies this is still the case?
Because (unfortunately) as far as I can tell, it is not.
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.
Just making sure I got the right idea here, are you saying to default on writeConfig
to the extension file and if this is blank then use the setting for config type?
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.
Yup. At least, that's compatible with the current behaviour, taking the original configType by default is not.
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.
Gotcha 👍
@sagikazarmark I added the requested changes, this complicated the test a bit as the read depends always of the |
@sagikazarmark any update on this PR? |
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.
@gssbzn Sorry for the delay, thanks again for the PR and the nice refactor!
The suppport introduced for files with no file extension is only partial as trying to save the config file would fail with
<file name> equires valid extensio
This adds support to saving such files