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

How to write odin info as json #48

Merged
merged 4 commits into from
Aug 29, 2024
Merged

Conversation

Hyrtwol
Copy link
Contributor

@Hyrtwol Hyrtwol commented Aug 8, 2024

  • Have you added the example to the CI at .github/workflows/check.yml?

@laytan
Copy link
Sponsor Collaborator

laytan commented Aug 26, 2024

There are a couple things I would change here:

  1. Don't use a string builder and just use json.marshal
  2. Handle the marshal error
  3. Exit with non-zero exit code if there is an error
  4. Use write_entire_file_or_err to get an actual error from the file creation instead of just a boolean
  5. Inline the marshal options

End result:

/*
Demonstrate how you can create a JSON file.
*/
package main

import "base:builtin"
import "base:runtime"

import "core:encoding/json"
import "core:fmt"
import "core:os"

main :: proc() {
	fmt.println("Some of Odin's builtin constants")

	path := len(os.args) > 1 ? os.args[1] : "odin_info.json"

	info: struct {
		ODIN_OS:      runtime.Odin_OS_Type,
		ODIN_ARCH:    runtime.Odin_Arch_Type,
		ODIN_ENDIAN:  runtime.Odin_Endian_Type,
		ODIN_VENDOR:  string,
		ODIN_VERSION: string,
		ODIN_ROOT:    string,
		ODIN_DEBUG:   bool,
	} = {ODIN_OS, ODIN_ARCH, ODIN_ENDIAN, ODIN_VENDOR, ODIN_VERSION, ODIN_ROOT, ODIN_DEBUG}

	fmt.println("Odin:")
	fmt.printfln("%#v", info)

	fmt.println()

	fmt.println("JSON:")
	json_data, err := json.marshal(info, {
		pretty         = true,
		use_enum_names = true,
	})
	if err != nil {
		fmt.eprintfln("Unable to marshal JSON: %v", err)
		os.exit(1)
	}

	fmt.printfln("%s", json_data)
	fmt.printfln("Writing: %s", path)
	werr := os.write_entire_file_or_err(path, json_data)
	if werr == nil {
		fmt.println("Done")
	} else {
		fmt.eprintfln("Unable to write file: %v", werr)
		os.exit(1)
	}
}

@Hyrtwol
Copy link
Contributor Author

Hyrtwol commented Aug 27, 2024

Tx for the feedback, looks cleaner :)
Tbh i wrote this snippet as one of the first things in odin :p
Not sure why i ended up using the strings.Builder, i remember i had a bit trouble finding a good example (thats why i made the pr :))
write_entire_file_or_err feels new? (or maybe i'm just blind) nice with an errorcode.
Just noticed marshal_to_writer so this example could also write the json data directly to a file... on that note would love an example using the Writer system, so i'll save that for another day.
will fix the changes asap :)

@laytan
Copy link
Sponsor Collaborator

laytan commented Aug 27, 2024

Yeah it is a good idea for an example! Indeed the or_err variants came a few weeks ago, not sure if you had written this already. And yep you could write directly to the file through the writers, but you wouldn't be able to print it like you did here so I think this is fine as is.

Writing directly to the file might also cause slow downs because you are doing a bunch of small writes/syscalls instead of writing to a memory buffer and then doing one write/syscall to the file. It is usually a trade-off between speed and memory usage and only really something to worry about on huge files. There is also core:bufio that you can use to buffer the writes which is another valid option. All have their use cases.

@Kelimion
Copy link
Member

Looks good. Can you remove the unused core:strings import as well? We can merge it after that.

@Kelimion Kelimion merged commit 794d8f7 into odin-lang:master Aug 29, 2024
1 check passed
@Hyrtwol Hyrtwol deleted the odin_info branch August 30, 2024 13: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