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

fix(lens): toml: write support #742

Merged
merged 7 commits into from
May 6, 2022
Merged

fix(lens): toml: write support #742

merged 7 commits into from
May 6, 2022

Conversation

g0hl1n
Copy link
Contributor

@g0hl1n g0hl1n commented Oct 26, 2021

The toml lens fails to save simple toml files due to problems with multi
dimensional (>2) arrays.
Therefore disable arrays with more than 2 dimensions for now to enable
basic usage again.

This commit fixes issue #715 by applying the suggested workaround. If
anyone can come up with a better solution please feel free to do so.

Fixes: 5e1cd31 (TOML lens (#91), 2018-11-29)
Signed-off-by: Richard Leitner [email protected]

@g0hl1n
Copy link
Contributor Author

g0hl1n commented Oct 26, 2021

If this gets merged please also adapt the NEWS file to mention arrays with more than 2 dimensions are not supported until further notice.

@g0hl1n g0hl1n changed the title fix(lenses): toml: make basic function work again fix(lenses): toml: write support Oct 26, 2021
The toml lens fails to save simple toml files due to problems with multi
dimensional (>2) arrays.
Therefore disable arrays with more than 2 dimensions for now to enable
basic usage again.

This commit fixes issue #715 by applying the suggested workaround. If
anyone can come up with a better solution please feel free to do so.

Fixes: 5e1cd31 (TOML lens (#91), 2018-11-29)
Signed-off-by: Richard Leitner <[email protected]>
toml.io now provides a versioned reference URL, therefore use this
instead of the old GitHub repo link.

Signed-off-by: Richard Leitner <[email protected]>
@g0hl1n g0hl1n changed the title fix(lenses): toml: write support fix(lens): toml: write support Oct 26, 2021
This test covers the bug described in issue #715.

Signed-off-by: Richard Leitner <[email protected]>
The "Test: Toml.array_norec; Array of arrays" tested a 3 dimensional
array. This commit fixes the test to actually test a 2 dimensional
array.

More than 2 dimensional arrays are currently not supported due to
issue #715.

Signed-off-by: Richard Leitner <[email protected]>
@g0hl1n
Copy link
Contributor Author

g0hl1n commented Nov 3, 2021

@raphink any questions or feedback on this PR?

@g0hl1n
Copy link
Contributor Author

g0hl1n commented Nov 18, 2021

[PING]
Any feedback/questions?
@georgehansper @lutter @raphink

@georgehansper
Copy link
Member

This solves the immediate problem, which is that the toml lens cannot save any files at all.
I'm happy to accept it as-is, and further improvements can be made at a later stage.

@raphink did you want to add/change anything ?

@g0hl1n
Copy link
Contributor Author

g0hl1n commented Dec 28, 2021

[PING]
@raphink any feedback from your side?

@g0hl1n
Copy link
Contributor Author

g0hl1n commented May 5, 2022

[PING]
@raphink, @georgehansper any open merge-blocking issues with this PR?

Copy link
Member

@georgehansper georgehansper left a comment

Choose a reason for hiding this comment

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

This change resolves the immediate problem of not being able to save toml files.

@georgehansper georgehansper merged commit a7e5296 into hercules-team:master May 6, 2022
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.

2 participants