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

feature: Have config.json pretty printed #275

Merged
merged 1 commit into from
May 22, 2024

Conversation

winksaville
Copy link
Contributor

Using jsonxf to pretty print config.json when saving.

@winksaville winksaville requested a review from macjuul as a code owner May 15, 2024 18:13
@winksaville
Copy link
Contributor Author

I believe having config.json pretty printed makes it more useful for humans.

I've chosen jsonxf simply because when searching crates.io it was most relevant and had almost 100,000 downloads. Another obvious choice would be to use serde_json::to_string_pretty but since serde wasn't being used currently I thought maybe there was a reason not to so looked for another alternative.

Using serde::to_string_pretty to format config.json when saving.
@winksaville
Copy link
Contributor Author

Rebased and altered this PR to build on main. In that surrealdb/src-tauri/Cargo.toml now has serde & serde_json as dependencies I switched to using serde_json::to_string_pretty, this simplifies the change and it's now only a two line change to surrealdb/src-tauri/src/config.rs.

@macjuul
Copy link
Contributor

macjuul commented May 22, 2024

LGTM! Thanks!

@macjuul macjuul merged commit 423c401 into surrealdb:main May 22, 2024
1 check passed
@winksaville winksaville deleted the pretty-print-config.json branch May 22, 2024 17:52
@macjuul
Copy link
Contributor

macjuul commented May 23, 2024

I have changed the implementation back to using jsonxf as serde caused the entire config to be saved as a single JSON string, corrupting it in the process 😅

@winksaville
Copy link
Contributor Author

winksaville commented May 23, 2024 via email

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.

None yet

2 participants