Skip to content

Conversation

@savil
Copy link
Collaborator

@savil savil commented Sep 8, 2022

Summary

We need this to parse Cargo.toml files.

I chose this library: github.com/pelletier/go-toml.

  • The alternative is https://github.com/BurntSushi/toml. This one has more github stars than go-toml.
  • go-toml's interface is more similar to encoding/json, and is also used by viper.

Also, changed v to value in cuecfg.go to address linter complaint.

How was it tested?

go test ./cuecfg/...

Copy link
Collaborator Author

savil commented Sep 8, 2022

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

Copy link
Collaborator Author

savil commented Sep 8, 2022

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@savil savil requested a review from loreto September 8, 2022 22:56
// TODO: consider using cue's JSON marshaller instead of
// "encoding/json" ... it might have extra functionality related
// to the cue language.
func MarshalToml(v interface{}) ([]byte, error) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I made this public (uppercase M in MarshalToml) to be consistent with the json and yaml versions.

IMO these should be private in the cuecfg package's interface. If folks agree, then I can send another PR to make them private.

cuecfg/toml.go Outdated

// TODO: consider using cue's JSON marshaller instead of
// "encoding/json" ... it might have extra functionality related
// to the cue language.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oops, this is copy-pasta from json.go. I'll remove this comment.

Copy link
Contributor

@loreto loreto left a comment

Choose a reason for hiding this comment

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

LGTM

You have the right library, because we're already using it in python_poetry_planner.go

},
}

var testTomlStr = `Version = 2
Copy link
Contributor

Choose a reason for hiding this comment

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

I find heredoc.Doc handy for these cases

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah, I like that

sadly, because of gofmt it ends up like this:

var testTomlStr = heredoc.Doc( `Version = 2 Name = 'go-toml' [Meta] Tags = ['go', 'toml'] `, ) 
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

which isn't much better IMO 🤷🏾 😭

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure?

Have you tried:

var testTomlStr = heredoc.Doc(` Version = 2 Name = 'go-toml' [Meta] Tags = ['go', 'toml'] `) 

(notice placement of the backticks)

@savil savil force-pushed the savil/cuecfg-toml branch from 75cf460 to a94c15b Compare September 9, 2022 04:01
@savil savil merged commit 26e7e82 into main Sep 9, 2022
@savil savil deleted the savil/cuecfg-toml branch September 9, 2022 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants