Skip to content
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 TOML support #66

Merged
merged 1 commit into from
Apr 1, 2017
Merged

Add TOML support #66

merged 1 commit into from
Apr 1, 2017

Conversation

n10v
Copy link
Contributor

@n10v n10v commented Mar 20, 2017

Fixes #61
Updates gohugoio/hugo#3200
Updates gohugoio/hugo#2577

@n10v n10v force-pushed the toml branch 4 times, most recently from 24467b2 to e39cf3a Compare March 20, 2017 07:24
@n10v
Copy link
Contributor Author

n10v commented Mar 20, 2017

There is one issue with go-toml: merge command can't marshal translations with this package.
I become this bug:
failed to marshal en-us strings to toml: cannot convert type *translation.template to TomlTree,
but *translation.template actually have a String method for these purposes.
Made corresponding issue for it: pelletier/go-toml#141

@n10v
Copy link
Contributor Author

n10v commented Mar 20, 2017

This should fix: pelletier/go-toml#142

@n10v
Copy link
Contributor Author

n10v commented Mar 20, 2017

/cc @moorereason @bep

README.md Outdated
@@ -118,6 +118,10 @@ Here is an example of the default file format that go-i18n supports:

To use a different file format, write a parser for the format and add the parsed translations using [AddTranslation](https://godoc.org/github.com/nicksnyder/go-i18n/i18n#AddTranslation).

TOML supports **only** flat format layout. About flat format you can read information below.

Choose a reason for hiding this comment

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

Note that TOML only supports the flat format, which is described below.

README.md Outdated
@@ -166,6 +170,8 @@ and name of substructures (ids) should be always a string.
If there is only one key in substructure and it is "other", then it's non-plural
translation, else plural.

More examples of flat format translation files you can find [here](https://github.com/nicksnyder/go-i18n/tree/master/goi18n/testdata/input/flat).

Choose a reason for hiding this comment

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

More examples of flat format translation files can be found in [testdata/input/flat](https://github.com/nicksnyder/go-i18n/tree/master/goi18n/testdata/input/flat).

return []translation.Translation{}, nil
}

ext := filepath.Ext(filename)

// `github.com/pelletier/go-toml` has unwonted unmarshal functions,

Choose a reason for hiding this comment

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

// `github.com/pelletier/go-toml` lacks an Unmarshal function,

Choose a reason for hiding this comment

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

Also, I would pull this logic out into a separate unmarshalTOML. Use it in unmarshal instead of in here.

return nil, err
}
s, err := tree.ToTomlString()
return []byte(s), err

Choose a reason for hiding this comment

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

I would pull this logic out into a marshalTOML(v interface{}) ([]bytes, error) function. Or submit a PR upstream.

@n10v
Copy link
Contributor Author

n10v commented Mar 21, 2017

@moorereason, thank you for your useful correctives. Everything is fixed.
And I understand I should study more english😓

@n10v
Copy link
Contributor Author

n10v commented Mar 21, 2017

Actually, I fixed pelletier/go-toml#141 so this PR is ready to merge!

@n10v
Copy link
Contributor Author

n10v commented Mar 26, 2017

Ping

@nicksnyder
Copy link
Owner

Thanks! Sorry again about the delay. Life is busy.

@nicksnyder nicksnyder merged commit fed5740 into nicksnyder:master Apr 1, 2017
@n10v n10v deleted the toml branch April 6, 2017 05:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants