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

Code cleanup: dart/ty.rs is accidentically modified #590

Closed
fzyzcjy opened this issue Jul 24, 2022 · 5 comments · Fixed by #603
Closed

Code cleanup: dart/ty.rs is accidentically modified #590

fzyzcjy opened this issue Jul 24, 2022 · 5 comments · Fixed by #603

Comments

@fzyzcjy
Copy link
Owner

fzyzcjy commented Jul 24, 2022

See #543 (comment) for details

/cc @lattice0 Maybe we can simply PR and revert this?

/cc @Desdaemon I also find this snippet in #589...

@lattice0
Copy link
Contributor

I'll take a better look on this once I finish the exceptions PR, cause I'm really bad at multitasking. Is it ok?

@fzyzcjy
Copy link
Owner Author

fzyzcjy commented Jul 26, 2022

Sure, take your time!

@Roms1383
Copy link
Contributor

I can come up with a PR if you want guys.
The only point of friction when applying the change is that in fn structs(&self) -> String current state, dart_api_class_name is explicitly set to None.

should None be preserved ?

either:

fn structs(&self) -> String {
    if let IrTypeDelegate::PrimitiveEnum { ir, .. } = &self.ir {
        super::TypeEnumRefGenerator {
            ir: ir.clone(),
            context: self.context.clone(),
        }
        .structs()
    } else {
        "".into()
    }
}

or:

fn structs(&self) -> String {
    if let IrTypeDelegate::PrimitiveEnum { ir, .. } = &self.ir {
        let mut context = self.context.clone();
        context.dart_api_class_name = None;
        super::TypeEnumRefGenerator {
            ir: ir.clone(),
            context,
        }
        .structs()
    } else {
        "".into()
    }
}

@fzyzcjy
Copy link
Owner Author

fzyzcjy commented Jul 30, 2022

@Roms1383 That would be great :)

The immutable approach looks better IMHO

@Roms1383 Roms1383 mentioned this issue Jul 30, 2022
3 tasks
@github-actions
Copy link
Contributor

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants