Feature/write config

[theherk]

This PR is based on earlier code submitted by @g3rk6 in #153. I believe I have satisfied all requests in @spf13's comments.

It adds methods to write the configuration back to the file or to a new file. There are methods to do so only if the file does not exist, as well

This PR has been alive, kept synchronized, and unmerged for a very long time. As a result, for those of you that need this feature, I am maintaining a synchronized fork here. This fork's master branch is based of the current tip here at sp313/viper, but with the addition of merged branches represented in #287 and #342. Please feel free to import github.com/theherk/viper if you need to use those features.

[CLAassistant]

All committers have signed the CLA.

[theherk]

This PR is no longer a work in progress. I believe it is ready to go. Based on the number of changes, it probably looks quite a bit bigger than it is. More specifically, that is due to moving the unmarshalReader to viper.go. That enables the storing of properties on the viper object to that comments could be written back to the file.

In summary what this does is add methods to safely or unsafely write the current configuration back to the currently configured config file or to a named config file. For example,

err := v.WriteConfig()
// or
err := v.WriteConfigAs("config.yaml")
// or
err := v.SafeWriteConfigAs("only_if_i_dont_exist.hcl")

In addition, the spelling of marshal is uniform across the library now.

This should resolve each of the issue requested by the author here.

[theherk]

I did have a test error that required an update, but everything else seems fine. I cannot determine why some of the builds are failing. It is complaining about some syntax in hcl/printer/nodes.go.

[theherk]

It is complaining about this line in hcl/printer/nodes.go. I don't understand that error. It seems defined to me.

Can you look into why this build is failing.

[theherk]

Okay, I sorted it out. bytes.ContainsRune was only added in Go 1.7. See the commit and the release notes. I'm going to work on build constraints.

[theherk]

Alright. Now I have it set up only to allow the writing of HCL files when using Go 1.7. All builds are passing. We should be good to go.

[moorereason]

I'd recommend that you submit a PR to hashicorp/hcl. ContainsRune is trivial.

-    if bytes.ContainsRune(result, '\n') {
+    if bytes.IndexRune(result, '\n') >= 0 {

[theherk]

Are you suggesting I do that in lieu of having some code supported differently based on the version in which it was compiled?

[theherk]

Okay, I have submitted the PR suggested: hashicorp/hcl#176. If that is accepted, I'll revert to the much better looking code.

[theherk]

Okay, that was a great solution. It was merged in by Hashicorp, so now we are back to the much prettier code, and builds are passing. 🏁

[theherk]

Also fixes #33 and fixes #113.

[andygrunwald]

Any update here @spf13 ?
I am looking forward for this feature, because i got a use case for this :)
Can i help here?

[bep]

@spf13 may have his opinions, but here are my 50 cents.

Most of it looks fine; but I think it is too much boilerplate code about file writing (which we could write a util func for, but it could also easily be written by the user).

The main funcs I want is:

func WriteConfig(f ConfigFormat, w io.Writer) error {
//...
}

It may be sensible in that func to check if that io.Writer is also an io.Closer and close it when done.

If also means that we must create proper and exported string consts for the different ConfigTypes, set that value on init (and export the value), and later we don't have to do the magic guessing by file name.

[theherk]

Forgive me @bep, I'm pretty new to Go, so I may not have done this in the most reasonable way, but I tried to follow the guidelines given by @spf13 here. Those include the request for a "safe" write method. It seemed like a good idea to me to allow the user to overwrite the current configuration or write a new one, which is why there are multiple methods.

I am probably misunderstanding your last statements, but I just used the SupportedExts as they were previously used. I didn't think it was too much magic to detect the config type based on the extension, but if you would prefer it another way, I made need further explanation. I thought this was a pretty good improvement over the previous incomplete PR, but I certainly don't want to disrupt the quality of the library.

[bep]

This is @spf13 library -- he should consider this PR. I just chimed in with my "50 cents", and I think that adding a lot of file handling into this feature is a low priority. An io.writer makes it flexible.

[theherk]

I'm going to maintain a fork of this with the write methods included. I hope some time @spf13 has time to check out the pull requests on viper again.

[dengwu12]

Needing this feature +1000000000, online waiting

[theherk]

@dengwu12: It looks like @spf13 may have abandoned maintenance entirely since his new gig. The other maintainer, @bep, has merged only two other PRs since this opened. I'm surprised given the popularity of Viper and Cobra how little love they are getting, but c'est la vie.

It seems that some conflicts have been generated that will require action to get this back to merge ready. In the meantime, I am maintaining the fork's master branch, because we required use of this feature. You can use it by importing from github.com/theherk/viper.

[spf13]

@theherk I've had health issues that have required me to seriously limit the time I could spend on things. I'm recovering and picking back up old issues.

I think the feature you've implemented is a must have for Viper. Can you bring the PR up to the latest and I'll do a full review of the code and we will get this merged in?

Thanks for the patience and for implementing this.

[andygrunwald]

@spf13 I hope you are doing well.

[theherk]

@spf13, thank you for following up. I'm sorry for any troubles you had, and I understand completely. I will get this ready for muster by the end of this weekend.

[theherk]

All modifications have been rebased onto the tip of master and conflicts are resolved. Tests are passing and everything seems to be working in my usage tests. Please let me know if you'd like me to do squashing. Otherwise, this is good to go.

[DaBlitzStein]

¿It is working now?

[theherk]

It is working, but has not been merged. @spf13 has been out for a while. I am maintaining a fork with this feature working. See this comment above.

[DaBlitzStein]

Supports remote @theherk ?

[theherk]

@DaBlitzStein: All of the commits that exist on master in spf13/viper exist on master in theherk/viper, with the addition of those from this PR. If you mean does it support writing back to a remote configuration, the answer is no. Interesting idea though, but that is a whole other can of worms. I'd rather get this PR in, before that feature is attempted.

[DaBlitzStein]

Thank you @theherk .

[Fjolnir-Dvorak]

Thank you @theherk. I am using your branch now. Hope it will get merged, soon, from one of the contributors.

[botherder]

I just wanted to +1 this feature. Thank you @theherk.

[dimitri]

I also need the feature, so I hope @spf13 is feeling better and able to get back to working on Viper. Thanks @theherk for maintaining your fork in the meanwhile!

[robdefeo]

What is the status of the PR it seems like such and incredibly useful feature that will save a lot of work for the community is there anything we can do to help @spf13

[segmentationtree]

what's the latest status on this? +1 on this feature.

[damianoneill]

Hi, is there any update on this PR?

Thanks.

[zaquestion]

@theherk can you fix the conflict on this? @spf13 is this otherwise good to merge?

[theherk]

Most definitely. I'll make sure it is squared away by the end of the weekend. But I haven't seen @spf13 comment on this for 7 months.

[zaquestion]

Might want to update the description as well, to clarify this is no longer a work in progress. Thanks for jumping on! Surely with some persistence we can get @spf13 to bring this in :)

[theherk]

Thank you @zaquestion. Updating the description was a great plan. I have sync'd the PR. Additionally, in answer to those of you needing to use this feature, I am maintaining a fork with those additions. See the PR description for details.

[zaquestion]

@bketelsen and @bep you 2 have been active most recently and appear to have merge rights. Can you review and merge these changes if appropriate? If not, can you get @spf13's attention?