Skip to content

Fixed wrong unsafe textures feature name in README #1431

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

Closed
wants to merge 1 commit into from

Conversation

ALEX11BR
Copy link

unsafe-textures => unsafe_textures

`unsafe-textures` => `unsafe_textures`
@@ -19,7 +19,7 @@ Rust-SDL2 uses the MIT license, but SDL2 itself is under the zlib license.
* `mixer` to link against SDL2\_mixer and have access to sound mixing features
* `ttf` to link against SDL2\_ttf and have access to various font features
* `raw-window-handle` to enable the crate `raw-window-handle`, which is useful to interop with various other backends.
* `unsafe-textures` to not have a lifetime in `Texture` structs. Texture are only freed when the program exits, or can be done manually through `unsafe`.
* `unsafe_textures` to not have a lifetime in `Texture` structs. Texture are only freed when the program exits, or can be done manually through `unsafe`.
Copy link
Contributor

Choose a reason for hiding this comment

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

hmmmm. This is inconsistent.

I'd prefer instead a sweeping change which only uses _ or only uses -, throughout.

It seems that the standard is to use underscore. The precedent is, the cargo book links to this example.

Please instead make everything be an '_'. @Cobrand @antonilol

Copy link
Author

Choose a reason for hiding this comment

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

I was referring to the fact that the feature is named unsafe_textures (with an underscore) as per the Cargo.toml file, while the README erroneously refers to it as unsafe-textures (with a hyphen).

This pull request only repairs the small error (typo) I found in the README.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that the standard is to use underscore.

I can confirm this, the cargo book has examples like the common no_std feature flag (so with an underscore)

Copy link
Contributor

@jagprog5 jagprog5 Mar 5, 2025

Choose a reason for hiding this comment

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

@ALEX11BR yes, I understand. However, that readme entry should not be changed. It is instead the name of the feature that should change, to make it match what is written in the readme.

The other features should be renamed as well. Please feel free to modify this PR. Or if you prefer, I can create it

@ALEX11BR
Copy link
Author

ALEX11BR commented Mar 5, 2025

that readme entry should not be changed

All right, I see.

As this is not my project, I'll let you handle the rename of the features and the potential complications.

@ALEX11BR ALEX11BR closed this Mar 5, 2025
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.

3 participants