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

[DOC] Main doc for encodings moved from encoding.c to doc/encodings.rdoc #5748

Merged
merged 2 commits into from
Apr 2, 2022
Merged

[DOC] Main doc for encodings moved from encoding.c to doc/encodings.rdoc #5748

merged 2 commits into from
Apr 2, 2022

Conversation

BurdetteLamar
Copy link
Member

A while back I merged doc/encoding.rdoc, which is a full discussion of encodings, meant to replace most of the discussion in encoding.c itself. This was discussed at length in PR 5578.

Now is the time to enact the replacement by removing some discussion from encoding.c, and instead linking to the new doc.

@BurdetteLamar BurdetteLamar added the Documentation Improvements to documentation. label Apr 1, 2022
@BurdetteLamar BurdetteLamar changed the title Main doc for encodings moved from encoding.c to doc/encodings.rdoc [DOC] Main doc for encodings moved from encoding.c to doc/encodings.rdoc Apr 1, 2022
Copy link
Contributor

@jeremyevans jeremyevans left a comment

Choose a reason for hiding this comment

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

Looks mostly good. Please see comments. OK to merge after the 3 comments have been addressed.

encoding.c Outdated
* "R\u00E9sum\u00E9"
* Encoding::ASCII_8BIT is a special-purpose encoding that is usually used for
* a string of bytes, not a string of characters.
* But as the name insists, its characters in the ASCII range
Copy link
Contributor

Choose a reason for hiding this comment

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

s/insists/indicates/

encoding.c Outdated
* a string of bytes, not a string of characters.
* But as the name insists, its characters in the ASCII range
* are considered as ASCII characters.
* This is useful when you use ASCII-8BIT characters with
Copy link
Contributor

Choose a reason for hiding this comment

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

s/This is useful.../This is useful when you use other ASCII-compatible encodings./

encoding.c Outdated
* external encoding must be specified to obtain the correct result.
* Encodings are important for Ruby strings.
* They are also important in other contexts: symbols, regexps, filesystems,
* locales, streams, and scripts.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would drop this paragraph. It's true that encodings are important for Ruby String, Symbol, Regexp, and IO instances. However, Ruby doesn't really have filesystems, locales, or scripts as core objects, so this reads awkwardly to me. I don't think the paragraph adds value, so I recommend removing it.

@BurdetteLamar BurdetteLamar merged commit 8174169 into ruby:master Apr 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Improvements to documentation.
2 participants