Skip to content

Bring init-exe main.zig in line with current style guidelines - #10697

Closed
ghost wants to merge 5 commits into
masterfrom
unknown repository
Closed

Bring init-exe main.zig in line with current style guidelines#10697
ghost wants to merge 5 commits into
masterfrom
unknown repository

Conversation

@ghost

@ghostghost commented Jan 26, 2022

Copy link
Copy Markdown
  • Removes anyerror
  • Uses proper output rather than logging

Sorry for the noise, but this was really bugging me.

- Removes `anyerror`
- Uses proper output rather than logging
Sorry for the noise, but this was *really* bugging me.
@marler8997

Copy link
Copy Markdown
Contributor

What's the reasoning you use to determine that going to stdout is proper here?

@ghost

ghost commented Jan 26, 2022 via email

Copy link
Copy Markdown
Author

Comment threadlib/std/special/init-exe/src/main.zig Outdated
@ghost

Copy link
Copy Markdown
Author

Whoops, the build is failing because the test case expects the output on stderr 😐 Then there seems to be an unrelated error.
None of the build logs are very helpful in finding the failed test; anyone know which one it is?

@nektro

Copy link
Copy Markdown
Contributor

@squeek502

Copy link
Copy Markdown
Member

@nektro

nektro commented Jan 26, 2022

Copy link
Copy Markdown
Contributor

this change would bring it into sync with the language reference https://ziglang.org/documentation/master/#Hello-World

@ghost

ghost commented Jan 26, 2022

Copy link
Copy Markdown
Author

I really do not agree with past ifreund on this matter. What we mean to do in this program is print a message, not log program behaviour. If we use the logging solution for the printing case, we give the impression that this is the correct way to do printing, which it is not.
Also, yes, it is strange that the docs don't match the template.
Oh, yeah, and I don't remember which of the links says this but the current impl won't print anything outside debug mode. Not a very robust program for a first impression.

@ghost

Copy link
Copy Markdown
Author

Looks like the only failed check was a timeout and not anything in the code itself. So it seems to all be fine.

Comment on lines +4 to +5
const stdout = std.io.getStdOut().writer();
try stdout.print("All your codebase are belong to us.\n", .{});

@andrewrkandrewrkJan 27, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Which one are we doing?

  1. Hello world as defined to be the simplest program that prints a string to stdout.
  2. Hello world as defined to be simplest program that debug prints, like you would do if you are troubleshooting something?

For (2) I could see it being a call to std.debug.print or std.log. But it looks like what you're trying to do is move this closer to (1), in which case this would be the way to do it:

Suggested change
conststdout=std.io.getStdOut().writer();
trystdout.print("All your codebase are belong to us.\n", .{});
trystd.io.getStdOut().writeAll("All your codebase are belong to us.\n");

I think that's not actually helpful though, because it would lead to people using this pattern instead of std.debug.print or std.log.

I think status quo is actually better.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I was doing
3. Hello world as defined to be the most correct (in terms of code style) program that prints a string to stdout.

The reasoning for this is, clearly what we're doing here is printing a message, no matter what mechanism we use; therefore what we do here will be taken as the correct way to print a message in Zig. We need to set a good example.

Here's why I say that status quo is not a good example:

  • It prints to stderr rather than stdout, in contrast to literally every other Hello World in existence.
  • It is not tolerant to errors, so misses prime advertising space for a headline feature of Zig.
  • It wraps the message in "info: "..'\n', which is not even what we want when doing a Hello World.
  • It does not run outside debug mode, so is not robust.

We're not playing code golf here; we're demonstrating, maybe for one of the first times to some people, an example of how to write Zig. We can be a bit more verbose for the sake of clarity, and I say we should.

@andrewrkandrewrkJan 28, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Most correct hello world is using writeAll, as suggested above. Your (3) is my (1). We are talking about the same thing.

zig init-exe is not intended to be "hello world" - it is intended to be a template to get you going. That's why it also includes unit tests. I think teaching people how to use std.log is more appropriate than teaching how to write to stdout, which is actually a rare use case.

I think it should be improved even further and set someone up a GeneralPurposeAllocator as well as an arena with the lifetime of the process, with comments explaining when someone might use one or the other.

Also argument parsing, and maybe even a buffered writer to write something to a file. Basically a "getting started" kit.

This PR is not the direction I see this going.

Sign up for freeto 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.

5 participants

@marler8997@nektro@squeek502@andrewrk@Vexu