Skip to content

AsciiChar::SOX should be AsciiChar::STX #78

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
chrismooredev opened this issue Feb 5, 2021 · 2 comments · Fixed by #92
Closed

AsciiChar::SOX should be AsciiChar::STX #78

chrismooredev opened this issue Feb 5, 2021 · 2 comments · Fixed by #92

Comments

@chrismooredev
Copy link

It seems that this crate's "Start of Text" member (ASCII '\x02') is mislabeled as SOX, whereas everything else on the web (including the linked-to Wikipedia page) references it as STX.

Since this would be a breaking change, I'm not sure how that would fit within the versioning, but I figured I'd report it anyway since I haven't seen any comments for this on the Issues/PRs for this repo.

@Zenithsiz
Copy link
Contributor

It should be possible to rename the variant itself and add a deprecated const item to the impl with the previous variant (or just add STX as the const item).
Something as such:

impl AsciiChar {
    #[deprecated("Use `AsciiChar::STX`")]
    pub const SOX: Self = Self::STX;
}

This should be technically non-breaking, even in match arms, as AsciiChar is both PartialEq and Eq, but not sure if it could break elsewhere.

@tormol
Copy link
Collaborator

tormol commented Jun 13, 2022

Renaming the variant breaks uses, both globs and explicit:

use ascii::AsciiChar::*; // Warning, unused.
let _ = SOX; // Error, did you mean SO?
use AsciiChar::SOX; // Error, did you mean SOH?

I couldn't find much about importing associated constants, other than a thread saying it's not supported and another thread explaining why. (the linked reference page doesn't make it clear at all).

I'll deprecate the variant, and then maybe rename with associated constant as fallback in a later version.

On the other hand I doubt which way it's done matters,
as I assume the only reason to glob-import will be to create a hardcoded &AsciiStr from &[AsciiChar] with printable characters. And useing select characters seems rather pointless.

tormol added a commit to tormol/rust-ascii that referenced this issue Jun 13, 2022
SOX is the wrong name. for this character.

Renaming the variant and adding SOX as associated constant
would break `use`s of it. both explicit and glob imports.

Fixes tomprogrammer#78.
tormol added a commit to tormol/rust-ascii that referenced this issue Jun 13, 2022
SOX is the wrong name. for this character.

Renaming the variant and adding SOX as associated constant
would break `use`s of it. both explicit and glob imports.

Fixes tomprogrammer#78.
tormol added a commit to tormol/rust-ascii that referenced this issue Sep 18, 2022
SOX is the wrong name for this character.

Renaming the variant and adding SOX as associated constant
would break `use`s of it. both explicit and glob imports.

Fixes tomprogrammer#78.
tormol added a commit that referenced this issue Sep 18, 2022
SOX is the wrong name for this character.

Renaming the variant and adding SOX as associated constant
would break `use`s of it. both explicit and glob imports.

Fixes #78.
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 a pull request may close this issue.

3 participants