Skip to content

Proposal: Require whitespace after a comma #8598

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

Closed
ve-nt opened this issue Apr 23, 2021 · 4 comments
Closed

Proposal: Require whitespace after a comma #8598

ve-nt opened this issue Apr 23, 2021 · 4 comments
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@ve-nt
Copy link

ve-nt commented Apr 23, 2021

Very similar to #7399. I propose similar measures in this case.

Here's a code example:

const std = @import("std");

pub fn main() !void {
    const nums = [_]f32{ 3.45, 8.23, 85.12, 3.2, 8,45, 8.11, 4.78 };
    for (nums) |n| std.debug.print("{}\n", .{n});
}

There is a bug in this code. How easy this is to spot will depend on the font and color scheme of your editor. Imagine that you weren't informed there was a bug, and there was a lot more code above and below this.

The answer is there is no 8.45 in nums. The author typo'd a , instead of a ..

zig-fmt will add whitespace after commas where needed, however this does not fix the bug, it only makes it more visible.

I can't say that I've personally seen this issue before in practice, but considering that . and , are neighbors on the vast majority of keyboard layouts, it doesn't seem unreasonable that something like this would happen.

I propose that whitespace be mandatory after a comma. Either a space or a newline. This will cause a compiler error in the above example and catch the bug before it becomes dormant. Likewise, zig-fmt should also error when it encounters the above code, as to not introduce the same (but more visible) footgun.

Here's some examples to make the new rule clearer:

const a = [_]f32{3.45,4.55,8.22,7.1}; // error
const b = [_]f32{3.45, 4.55,8.22, 7.1}; // error
const c = [_]f32{3.45, 4.55, 8.22, 7.1}; // okay
const d = [_]f32{ 3.45,    4.55,   8.22,   7.1 }; // okay
const e = [_]f32{
    3.45,
    4.55,
    8.22,
    7.1,
}; // okay
@Vexu Vexu added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Apr 23, 2021
@Vexu Vexu added this to the 0.9.0 milestone Apr 23, 2021
@kubkon
Copy link
Member

kubkon commented Apr 24, 2021

This is generally enforced by zig fmt so I don't really see any point in making this a hard error. Also, I'd imagine there'd be cases where you'd want to leave out the whitespaces altogether.

@andrewrk andrewrk modified the milestones: 0.9.0, 0.8.0 Apr 24, 2021
@andrewrk
Copy link
Member

To make something an error, zig fmt has to reject it rather than reformat it. In this case we want zig fmt to reformat it.

@marler8997
Copy link
Contributor

@andrewrk, so zig fmt won't be able to fix errors?

@SpexGuy
Copy link
Contributor

SpexGuy commented Apr 24, 2021

In the long term, yeah, zig fmt will reject syntax errors. In the short term we have it correcting breaking changes in the language, but post-1.0 it won't do that anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

6 participants