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

fmt: unify and align wording regarding characters, digits, and bytes #18315

Merged
merged 1 commit into from Sep 1, 2022

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jul 18, 2022

Contribution description

The current wording of the fmt module may be a bit confusing (see #18310). This PR aims to fix that.

Testing procedure

Read.

Issues/PRs references

Alternative to #18310

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: doc Area: Documentation labels Jul 18, 2022
@miri64 miri64 requested a review from kaspar030 July 18, 2022 08:40
@github-actions github-actions bot added Area: sys Area: System and removed Area: doc Area: Documentation labels Jul 18, 2022
@kaspar030
Copy link
Contributor

Can we postpone this until I'm back from holidays? No time to argue now :)
I find using "characters" utterly confusing. We're in total "one byte has eight bits" land, and so, fmt writes bytes to buffers. (It maybe prints characters).

@miri64
Copy link
Member Author

miri64 commented Jul 18, 2022

I find using "characters" utterly confusing. We're in total "one byte has eight bits" land, and so, fmt writes bytes to buffers.

Mh... the type of all out parameters for which I changed to "characters" are of type char *. Typically, in C this is a string (of characters) not a buffer (of bytes) so I do not understand your argument.

Can we postpone this until I'm back from holidays? No time to argue now :)

Sure!

@benpicco
Copy link
Contributor

Huh what's confusing about using characters for string output?
I wouldn't have expected this to be controversial.

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 1, 2022
@kaspar030
Copy link
Contributor

Huh what's confusing about using characters for string output?

because characters nowadays often mean multi-byte characters. 'Į' is a character.
When talking about how much data is written to a byte buffer, numbering "bytes" is just more precise.

@miri64
Copy link
Member Author

miri64 commented Sep 1, 2022

because characters nowadays often mean multi-byte characters. 'Į' is a character.
When talking about how much data is written to a byte buffer, numbering "bytes" is just more precise.

But that's not how these functions work. They provide char as an output/input, not 8-bit wide units, commonly called bytes (which is uint8_t in C). The width of char is platform dependent (though it is commonly understood it is also 8-bit ASCII characters). What you are talking about is UTF-8 characters, but those are a completely different type in C. Lastly, since this is about converting between bytes and their string representation, sticking to "bytes" for both input and output is confusing.

@kaspar030
Copy link
Contributor

ok

@kaspar030 kaspar030 merged commit fcf3e01 into RIOT-OS:master Sep 1, 2022
@miri64 miri64 deleted the fmt/doc/wording branch September 1, 2022 20:09
@maribu maribu added this to the Release 2022.10 milestone Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants