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

Cleanup unused imports #30

Merged
merged 1 commit into from
Sep 3, 2024
Merged

Conversation

tusharsadhwani
Copy link
Contributor

I wanted to build an unused import cleaner for Zig, and I ended up using this project as the test-bed for cleaning unused imports and finding edge cases in my implementation.

I'm not sure if such patches are welcome, but I'll just leave a PR here still.

All the test still pass after this changeset, so this shouldn't break anything :)

Copy link
Owner

@sin-ack sin-ack left a comment

Choose a reason for hiding this comment

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

Hey, thanks for this! Your tool looks really nice, I'm thinking of making it a pre-commit linter + CI check. One minor issue I spotted is that it removes empty lines after deleted imports aggressively. I think it should only remove a blank line after an import if there are no more imports left (i.e. removing the lines results in double empty lines); otherwise it should preserve the empty lines.

src/runtime/RuntimeError.zig Outdated Show resolved Hide resolved
src/runtime/map_builder.zig Outdated Show resolved Hide resolved
@tusharsadhwani
Copy link
Contributor Author

@sin-ack that has been taken care of, plus now the tool handles all unused global declarations, not just imports.

Copy link
Owner

@sin-ack sin-ack left a comment

Choose a reason for hiding this comment

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

Thanks, very cool!

@sin-ack sin-ack merged commit 14aa1c9 into sin-ack:master Sep 3, 2024
2 checks passed
@tusharsadhwani tusharsadhwani deleted the unused-imports branch September 3, 2024 09:48
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.

2 participants