-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
'zig fmt: on' directive not working inside array declaration #10418
Comments
This happens because The reason that const foo = .{
"a", "b",
"ccc", "d",
}; Sadly, even with this current complexity it fails badly when faced with the much greater complexity of unicode: const foo = .{
"a", "b",
"👨👩👧👦", "d",
}; I propose that we remove this auto alignment code in // zig fmt: off
const foo = .{
"a", "b",
"👨👩👧👦", "d",
};
// zig fmt: on |
column-cased alignment. This fixes multiple issues caused by bypassing the renderToken() infrastructure, as described in ziglang#10418
If we put the directive inside the array declarations, `zig fmt: on` does not work. For detail see ziglang/zig#10418
This had a number of problems, including: 1. Complexity: the code was complicated and it was the only place where allocation is used. Now zig token rendering is free of heap allocation. 2. Unicode awareness: handling Unicode properly will probably look like this: https://github.com/unicode-rs/unicode-width/blob/f5a9b4efa5142d20d5f23070542e46e76f426a30/src/tables.rs I'm not sure we want that too. Or do we only want basic Unicode awareness then? Problem is there will always be new issues "oh for these characters the width is wrong though". And I don't think adding those tables alone would be the end of this story, either. 3. Inconsistency: if I can do ``` _ = .{ x, y, z, w, }; ``` , why can't I also do ``` func( 1, 1, 1, 1, ); ``` ? These rules weren't applied consistently at all. Fixes ziglang#10418 Fixes ziglang#15929
I attempted this but the problem is not fixing the issue itself but rather the migration to the new zig fmt rules. The diff after Also, the zig codebase is not going to be the only place where array inits rely on this auto alignment. I think what we need to do is for one release we need to detect code array inits like this that rely on this auto alignment code and then add https://github.com/r00ster91/zig/commits/fmtlist is my branch. See top 3 commits. We might also want to change |
What about being explicit (like with As an example: //# zig fmt on will generate a |
Zig Version
0.10.0-dev.63+6b8e33d14
Steps to Reproduce
Expected Behavior
Expected
// zig fmt: on
to re-enable formattingActual Behavior
The
zig fmt: on
directive seems to be ignored inside a static struct/array declaration, even whenzig fmt: off
took effect inside it.The text was updated successfully, but these errors were encountered: