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

Fix clippy never_loop #14413

Closed
wants to merge 1 commit into from

Conversation

hecatia-elegua
Copy link
Contributor

@hecatia-elegua hecatia-elegua commented Mar 26, 2023

just a small fix / refactor since clippy errored out
also passes cargo fmt now

edit: I just realized clippy has more complaints, but I'll just keep the PR this way and go on with my actual problem

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 26, 2023
};
},
inner()
Copy link
Member

Choose a reason for hiding this comment

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

I take it rustfmt does the same thing? We should probably keep the comment.

Copy link
Member

@Veykril Veykril left a comment

Choose a reason for hiding this comment

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

Instead of turning this into a closure we can just remove the loop keyword and make use of the now stable label_break_value feature instead

@HKalbasi
Copy link
Member

It hits rust-lang/rustfmt#5676 but we can #[rustfmt::skip] that block, or just do nothing until that issue becomes resolved.

@hecatia-elegua
Copy link
Contributor Author

The problem was that clippy errors on this. Also it's kinda more readable, though if the closure is less performant or sth I understand

@Veykril
Copy link
Member

Veykril commented Mar 26, 2023

Using label_break_value seems more readable to me than a closure that is called immedaitely, closures usually imply being lazy to me when used in such a way (or emulating a try block).

@HKalbasi
Copy link
Member

@hecatia-elegua clippy is not considered a problem in this repository, but for maximizing readability I think you can make it a standalone function or method which is more readable than a labeled block and closure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants