Conversation
Signed-off-by: João Fernandes <joao@bckground.com>
Signed-off-by: João Fernandes <joao@bckground.com>
Signed-off-by: João Fernandes <joao@bckground.com>
Signed-off-by: João Fernandes <joao@bckground.com>
Signed-off-by: João Fernandes <joao@bckground.com>
e3b5072 to
d0bf7b4
Compare
d0bf7b4 to
39cd216
Compare
felixfontein
left a comment
There was a problem hiding this comment.
Thanks for picking this up! It would be great if this store could also handle comments, but it seems the library doesn't really handle them yet.
|
|
||
| // Sort each group independently. | ||
| sortKeysNaturally(simpleKeys) | ||
| sortKeysNaturally(complexKeys) |
There was a problem hiding this comment.
I don't think it is a good idea to sort keys. I as a SOPS user expect a store to not modify the key order.
| // mapToTreeBranch converts a map[string]any to a sops.TreeBranch. | ||
| func mapToTreeBranch(m map[string]any) (sops.TreeBranch, error) { | ||
| // Separate keys by type: simple values first, then complex types | ||
| // (tables/arrays). |
There was a problem hiding this comment.
What is the rationale for this?
| } | ||
|
|
||
| // Replace single quotes with double quotes for string values. | ||
| result := bytes.ReplaceAll(buf.Bytes(), []byte("'"), []byte("\"")) |
|
|
||
| [2] | ||
| 21 = [21.1, 21.2] | ||
| 22 = 22 |
There was a problem hiding this comment.
Is it possible to make the indentation configurable?
There was a problem hiding this comment.
Unfortunately, no, when using the "normal" API. Please check #2031 (comment) - it might be possible if I go for a different approach.
Happy to contribute! I have good and bad news: the good news is that, to some extent, it's possible to handle comments. Being specific, it's possible to handle start-of-the-line comments, but not end-of-the-line comments, i.e., # We can handle these
xpto = 42 # But we can't handle theseThe bad news is that this requires using the unstable API and results in significantly more complex code. Should I give it a try? |
|
Hmm, good question... Do you know whether the marshaller supports comments (at least start-of-line ones) as well? If it does not, then I guess it doesn't make sense - yet - to support them. |
The marshaler, i.e., the "normal" |
|
I can only see parsing code in the unstable API. Can you point me to the unstable API that allows creating TOML files with start-of-the-line comments? |
|
Is this patch going to be merged? I need this badly to onboard sops :) Otherwise I will have to find alternatives for my toml files. |
|
That depends. Right now the PR is not mergable because not all commits are signed off. I'm also curious on the answer to my question, I was not able to find code in the library that allows to create TOML files with comments (but then, I didn't search very intensely). |
|
@jcmfernandes @bckground was deleting the branch (which seems to have been a consequence of deleting the repository) intentionally? |
I'm deeply sorry, @felixfontein. My associate deleted the repo from our org as he didn't know I was working on this. He owes us all a drink! I have good and bad news. The good news is that I was able to rewrite the TOML store and have it keep structure (order) and comments. Before I open a PR, please check https://github.com/bckground/sops/commits/toml-alternative and let me know what you think (disclaimer: AI-assisted but not AI-written). The bad news is that I had to modify https://github.com/pelletier/go-toml and my changes aren't yet upstream — you can find them at https://github.com/bckground/go-toml/commits/marshal-ast — and I had to use the I'm hopeful my changes will be accepted. I'll then attempt to get this TOML store merged. |
|
@jcmfernandes thanks for the update! I glanced over the code changes (both for SOPS and go-toml), and I think it would be great to have them. (I haven't looked in detail, and obviously I can't speak for go-toml whether they want them.)
I think it would benefit go-toml a lot to have a more general formatter API, even if it's "just" in unstable. Maybe this would also bring "unstable" one more step in the direction of eventually becoming stable? I saw you've started with a first PR: pelletier/go-toml#1031 🎉 A general note: I'd probably also add some "round-trip" tests for parser + formatter, where you have a set of TOML documents that are first parsed to an AST, and then the AST is formatted, and the result is compared to the original input. This obviously won't work for all TOML files, but it can help a lot during development to see what a change is causing (resp. that a change is not breaking anything on a higher level). |
Fixes #369. Supersedes #812 (that in turn superseded #533). Hopefully, the third time's the charm.
Uses
github.com/pelletier/go-toml/v2. It discards comments.