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

zig fmt incorrectly allows extra newlines at end of file #2074

Closed
andrewrk opened this Issue Mar 18, 2019 · 2 comments

Comments

Projects
None yet
2 participants
@andrewrk
Copy link
Member

andrewrk commented Mar 18, 2019

Here's a (currently failing) test to go in std/zig/parser_test.zig:

test "zig fmt: extra newlines at the end" {
    try testTransform(
        \\const a = b;
        \\
        \\
        \\
    ,
        \\const a = b;
        \\
    );
}
@shawnl

This comment has been minimized.

Copy link
Contributor

shawnl commented Mar 21, 2019

How should zig fmt handle extra newlines inside the file? Should reduce more than two adjacent newlines to two, except at the end, where only one is allowed?

@andrewrk

This comment has been minimized.

Copy link
Member Author

andrewrk commented Mar 21, 2019

Should reduce more than two adjacent newlines to two, except at the end, where only one is allowed?

Correct. This behavior is already implemented, except for the end. I believe the problem is actually in the detection of changes:

zig/std/zig/render.zig

Lines 36 to 62 in b5cc92f

if (!self.anything_changed_ptr.*) {
const end = self.source_index + bytes.len;
if (end > self.source.len) {
self.anything_changed_ptr.* = true;
} else {
const src_slice = self.source[self.source_index..end];
self.source_index += bytes.len;
if (!mem.eql(u8, bytes, src_slice)) {
self.anything_changed_ptr.* = true;
}
}
}
try self.child_stream.write(bytes);
}
};
var my_stream = MyStream{
.stream = MyStream.Stream{ .writeFn = MyStream.write },
.child_stream = stream,
.anything_changed_ptr = &anything_changed,
.source_index = 0,
.source = tree.source,
};
try renderRoot(allocator, &my_stream.stream, tree);
return anything_changed;

I think this logic is simply missing a check for source_index not matching source.len and marking anything_changed as true in this case.

andrewrk added a commit that referenced this issue Mar 26, 2019

fmt: check for extra newline at end of file
`anything_changed` checks if `source_index` == `source.len`
Fixes #2074

@andrewrk andrewrk modified the milestones: 0.5.0, 0.4.0 Apr 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.