Skip to content

Conversation

DJawna
Copy link
Contributor

@DJawna DJawna commented Oct 22, 2024

This PR will add Log prioriyt dependent logging functions:

  • log_*

where * is: create, warning, error etc.

In addition to the message those new functions will take the Category as paraemter,
the Category needs to be converted to c_int, if that fails, Application value is taken and assumed it will be convertible to c_int.

@Cobrand
Copy link
Member

Cobrand commented Oct 24, 2024

Thanks, I will have a look once you fix my comment. Also don't forget to run cargo fmt, I'm pretty sure it fails your PR as-is.

@Cobrand
Copy link
Member

Cobrand commented Oct 28, 2024

The closure thing is very messy, just use Priority and pattern matching to figure out which function to call, don't use closures like that, it's quite hard to read for very little gain.

function which accepts priority enum as parameter
@DJawna
Copy link
Contributor Author

DJawna commented Oct 28, 2024

The closure thing is very messy, just use Priority and pattern matching to figure out which function to call, don't use closures like that, it's quite hard to read for very little gain.

Now i have replaced all those function wrappers with the closures in them with just one single pub function which accepts the priority enum and matches it to invoke the unsafe sdl log functions

@Cobrand
Copy link
Member

Cobrand commented Oct 28, 2024

Much better. I think you can still add log_debug log_critical etc that are just calls to log_with_category inside for simplicity.

which call the generic logging function
@DJawna
Copy link
Contributor Author

DJawna commented Oct 28, 2024

Much better. I think you can still add log_debug log_critical etc that are just calls to log_with_category inside for simplicity.

Ok did that now!

@Cobrand Cobrand merged commit 07ae517 into Rust-SDL2:master Feb 18, 2025
17 checks passed
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