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 ignored clippy lints #12160

Merged
merged 2 commits into from Mar 11, 2024
Merged

Conversation

IanManske
Copy link
Member

Description

Fixes some ignored clippy lints.

User-Facing Changes

Changes some signatures and return types to &dyn Command instead of &Box<dyn Command, but I believe this is only an internal change.

@IanManske IanManske changed the title Fix ignored clippy Fix ignored clippy lints Mar 11, 2024
} else if i == chars.len() {
true
// `**` ends in non-separator
match count.cmp(&2) {
Copy link
Member

Choose a reason for hiding this comment

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

Not quite sure if this monster gets much more readable. maybe worth pullilng the Less branch up?

@@ -788,11 +787,11 @@ impl EngineState {
self.vars[var_id].const_val = Some(val);
}

#[allow(clippy::borrowed_box)]
pub fn get_decl(&self, decl_id: DeclId) -> &Box<dyn Command> {
pub fn get_decl(&self, decl_id: DeclId) -> &dyn Command {
Copy link
Member

Choose a reason for hiding this comment

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

Oh yes (walked past that today SMH)

@@ -48,7 +48,6 @@ pub fn build_table(value: Value, description: String, termsize: usize) -> String

add_padding_to_widths(&mut widths);

#[allow(clippy::manual_clamp)]
Copy link
Member

Choose a reason for hiding this comment

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

Ah they made that generally allow again. I think this is only an undefined resolution for float. So the code below could be a safe clamp, right?

Copy link
Member

Choose a reason for hiding this comment

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

Ah the .max/.min solution never panics.

@sholderbach sholderbach merged commit 26786a7 into nushell:main Mar 11, 2024
15 checks passed
@hustcer hustcer added this to the v0.92.0 milestone Mar 12, 2024
@IanManske IanManske deleted the fix-ignored-clippy branch March 26, 2024 14:54
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.

None yet

3 participants