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] Enhanced RDoc for IO #6669

Merged
merged 5 commits into from Nov 9, 2022
Merged

[DOC] Enhanced RDoc for IO #6669

merged 5 commits into from Nov 9, 2022

Conversation

BurdetteLamar
Copy link
Member

This resolves discrepancies between io.c and doc/io_streams.rdoc. By previous agreement, examples for methods belong in the former; the latter may have examples for certain values and concepts.

This also improves the doc for line number, which previously was unclear and incomplete.

@BurdetteLamar BurdetteLamar added the Documentation Improvements to documentation. label Nov 3, 2022
Copy link
Member

@peterzhu2118 peterzhu2118 left a comment

Choose a reason for hiding this comment

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

While I think improving the docs for IO is very much needed, I'm not sure if I see the value for some of these sections in io_streams.rdoc. I personally find linking to different pages in documentation difficult to follow and less time-efficient to read. Here are some examples:

  • "Line Options": There's only a single keyword argument listed. IMO making the reader go to another page just to read a single sentence is counterintuitive and would save them so much time if it was just embedded in the docs for IO.
  • Most sections like "Basic IO", "End-of-Stream", "Line IO", "Line Separator", "Character IO", "Byte IO", "Codepoint IO", etc. are basically another way to group "What's Here" sections. Maybe it should belong in the docs of IO?

- IO#read: Returns all remaining or the next _n_ bytes read from the stream,
for a given _n_:
- IO#read: Reads and returns some or all of the remaining bytes from the stream.
- IO#write: Writes zero or more strings to the stream, derived from the given objects:
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what "derived from the given objects" means. Could you clarify?

Copy link
Member Author

Choose a reason for hiding this comment

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

Modified (clarified?).

f.lineno # => 0
f.close
- IO#tell (aliased as +#pos+): Returns the current position (in bytes) in the stream.
- IO#pos=: Sets the position of the stream to a given integer +new_position_ (in bytes):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- IO#pos=: Sets the position of the stream to a given integer +new_position_ (in bytes):
- IO#pos=: Sets the position of the stream to a given integer +new_position_ (in bytes).

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.


f = File.new('t.txt')
f.eof? # => false
f.read # => "First line\nSecond line\n\nFourth line\nFifth line\n"
f.eof? # => true

Or by using method IO#seek:
Copy link
Member

Choose a reason for hiding this comment

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

I prefer this old format that separates the two examples. I find longer examples to be hard to read. I also prefer if we showed IO#seek first since it avoids reading all the contents of the file (which can be slow if the file is very large).

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

io.c Outdated
* Sets and returns the line number for the stream.
* See {Line Number}[rdoc-ref:io_streams.rdoc@Line+Number].
* Sets and returns the line number for the stream;
* see {Line Number}[rdoc-ref:io_streams.rdoc@Line+Number]:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* see {Line Number}[rdoc-ref:io_streams.rdoc@Line+Number]:
* see {Line Number}[rdoc-ref:io_streams.rdoc@Line+Number].

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

io.c Outdated
* returns +self+.
* does nothing if already at end-of-stream;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* does nothing if already at end-of-stream;
* Does nothing if already at end-of-stream;

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

io.c Outdated
Comment on lines 4946 to 4957
* File.read('t.dat')
* # => "\xFE\xFF\x99\x90\x99\x91\x99\x92\x99\x93\x99\x94"
* File.read('t.dat')
* # => "\xFE\xFF\x99\x90\x99\x91\x99\x92\x99\x93\x99\x94"
* f = File.new('t.dat')
* f.getbyte # => 254
* f.getbyte # => 255
* f.seek(-2, :END)
* f.getbyte # => 153
* f.getbyte # => 148
* f.getbyte # => nil
* f.close
Copy link
Member

Choose a reason for hiding this comment

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

What's the purpose of adding these examples? Adding this makes the examples very long, could we shorten it with the example above?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed.

f.rewind
f.seek(0, :SET)
$. # => 5
$. # => 5
Copy link
Member

Choose a reason for hiding this comment

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

Why is $. outputted twice here?

Copy link
Member Author

Choose a reason for hiding this comment

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

One removed.

@BurdetteLamar
Copy link
Member Author

* "Line Options": There's only a single keyword argument listed. IMO making the reader go to another page just to read a single sentence is counterintuitive and would save them so much time if it was just embedded in the docs for IO.

Section and links thereto removed.

@BurdetteLamar
Copy link
Member Author

* Most sections like "Basic IO", "End-of-Stream", "Line IO", "Line Separator", "Character IO", "Byte IO", "Codepoint IO", etc. are basically another way to group "What's Here" sections. Maybe it should belong in the docs of `IO`?

I'd planned to link to these sections from ARGF, IO, StringIO, Kernel, and possibly File. I put them here b/c they cite methods common to (almost) all. I've considered linking from these sections to methods in all the classes (instead of saying what they're in and not in; thoughts?

@peterzhu2118
Copy link
Member

I think most readers would be reading for IO rather than any of the other classes, so if we could avoid linking out in IO`and instead link out to IO in the other classes, then we could at least improve the reading experience in IO. I also find the "(also in ABC)" and "(not in XYZ)" parts rather difficult to follow, if we linked to IO from these other classes, we could avoid doing this.

@BurdetteLamar
Copy link
Member Author

I think most readers would be reading for IO rather than any of the other classes, so if we could avoid linking out in IO`and instead link out to IO in the other classes, then we could at least improve the reading experience in IO.

I don't envision readers repeatedly following individual links from class IO to page IO Streams. Instead, I envision them landing on page IO Streams a first or second time, then reading deeply there.

I think the discussion of IO streams of all kinds has value. There's no other place in the doc where the reader can get a sense of a family of methods such as those in sections "Position," "Open and Closed Steams," "Line IO," and "Character IO."

I also find the "(also in ABC)" and "(not in XYZ)" parts rather difficult to follow, if we linked to IO from these other classes, we could avoid doing this.

I've planned on adding this table, which would (trivially) require changing the page from .rdoc to .md. Note that the table cannot be added to io.c (it's not markdown):

| Method         | ARGF | IO/File | StringIO | Kernel |
|:---------------|------|---------|----------|--------|
| close          | x    | x       | x        |        |
| close_read     |      | x       | x        |        |
| close_write    |      | x       | x        |        |
| closed?        | x    | x       | x        |        |
| each           | x    | x       | x        |        |
| each_byte      | x    | x       | x        |        |
| each_char      | x    | x       | x        |        |
| each_codepoint | x    | x       | x        |        |
| each_line      | x    | x       | x        |        |
| eof            | x    | x       | x        |        |
| eof?           | x    | x       | x        |        |
| getbyte        | x    | x       | x        |        |
| getc           | x    | x       | x        |        |
| gets           | x    | x       | x        | x      |
| lineno         | x    | x       | x        |        |
| lineno=        | x    | x       | x        |        |
| pos            | x    | x       | x        |        |
| pos=           | x    | x       | x        |        |
| putc           | x    | x       | x        | x      |
| puts           | x    | x       |          | x      |
| read           | x    | x       | x        |        |
| read_nonblock  | x    | x       |          |        |
| readbyte       | x    | x       |          |        |
| readchar       | x    | x       |          |        |
| readline       | x    | x       |          | x      |
| readlines      | x    | x       | x        | x      |
| readpartial    | x    | x       |          |        |
| reopen         |      | x       | x        |        |
| rewind         | x    | x       | x        |        |
| seek           | x    | x       | x        |        |
| tell           | x    | x       | x        |        |
| ungetbyte      |      | x       | x        |        |
| ungetc         |      | x       | x        |        |
| write          | x    | x       | x        |        |

@peterzhu2118
Copy link
Member

For me, when I read documentation, I want it to be the source of truth with all of the information available. I usually have an idea of what I'm looking for so it's important to me that most (or all) of the information about the class is available on a single page so it's easy to read and search.

I think the discussion of IO streams of all kinds has value. There's no other place in the doc where the reader can get a sense of a family of methods such as those in sections "Position," "Open and Closed Steams," "Line IO," and "Character IO."

Again, this sounds like a "What's here" section that could live in the documentation for the respective classes. Discussion about specifically what "Line IO" or "Character IO" means could live in the documentation in IO and linked to from other classes (e.g. StringIO or ARGF). Another way would be to create small rdoc fragments and each of these classes could use an :include: directive to include them rather than linking.

Again, my main problem here is that this document includes a list of methods where most of it links to methods in IO and so it may or may not exist in the class they're looking at

I've planned on adding this table, which would (trivially) require changing the page from .rdoc to .md. Note that the table cannot be added to io.c (it's not markdown):

I'm not sure what the value of the table is. If I wanted to look at what methods are available for the class, there's a summary on the left hand side of the documentation of the class with the methods of that class.

@BurdetteLamar
Copy link
Member Author

So why have the IO Stream page at all?

@peterzhu2118
Copy link
Member

I'm saying we could move many of the parts of this document into the documentation in IO and link to IO from the other classes. It should at least improve the experience of reading documentation in IO.

@BurdetteLamar
Copy link
Member Author

I'm saying we could move many of the parts of this document into the documentation in IO and link to IO from the other classes. It should at least improve the experience of reading documentation in IO.

So what, if anything, should remain on the io_streams page?

@peterzhu2118
Copy link
Member

I think most of these can be moved to the docs for the IO class. But I can only give my opinion from how I use the docs, and you may have other opinions.

@BurdetteLamar
Copy link
Member Author

I think most of these can be moved to the docs for the IO class. But I can only give my opinion from how I use the docs, and you may have other opinions.

I'll plan to merge io_stream.rdoc into io.c.

@BurdetteLamar
Copy link
Member Author

@peterzhu2118, ready for you.

Copy link
Member

@peterzhu2118 peterzhu2118 left a comment

Choose a reason for hiding this comment

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

I really like this change, thank you for making the change!

I think there's links to io_streams.rdoc in file.c and stringio.c that should be updated to make sure there's no dead links.

io.c Show resolved Hide resolved
io.c Outdated Show resolved Hide resolved
io.c Outdated Show resolved Hide resolved
io.c Show resolved Hide resolved
@BurdetteLamar
Copy link
Member Author

I really like this change, thank you for making the change!

I think there's links to io_streams.rdoc in file.c and stringio.c that should be updated to make sure there's no dead links.

Fixed in file.c here. stringio.c needs separate PR in separate repository; working on it.

@BurdetteLamar
Copy link
Member Author

@peterzhu2118, I'm thinking to have free-standing doc for StringIO and ARGF -- i.e., not linking to IO. Their code is separate, and could conceivably behave differently.

Copy link
Member

@peterzhu2118 peterzhu2118 left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for addressing the comments!

@peterzhu2118
Copy link
Member

Can you give some examples about how they behave differently? I would expect behaviour differences to be explained in their respective documentation.

@BurdetteLamar
Copy link
Member Author

Can you give some examples about how they behave differently? I would expect behaviour differences to be explained in their respective documentation.

Not aware of any differences. But I'd like the docs to be free-standing anyway; I'm thinking of sections in class doc:

  • Basic IO
  • Position
  • Open and Closed Streams
  • End-of-Stream
  • Line IO
  • Character IO
  • Byte IO
  • Codepoint IO

For reasons you've given elsewhere (searchability, etc.), the link targets should be local. And the examples should show the local class in action, not link to examples using a different class.

@BurdetteLamar BurdetteLamar merged commit 0e1e1b1 into ruby:master Nov 9, 2022
@peterzhu2118
Copy link
Member

I think it makes more sense for most of the documentation to live in IO because although StringIO and ARGF isn't aren't IO, it's designed to look like IO as much as possible. And for that reason, I think readers should read documentation of IO to have a good understanding of how StringIO/ARGF works.

@BurdetteLamar
Copy link
Member Author

I think it makes more sense for most of the documentation to live in IO because although StringIO and ARGF isn't aren't IO, it's designed to look like IO as much as possible. And for that reason, I think readers should read documentation of IO to have a good understanding of how StringIO/ARGF works.

I do not agree. I think readers should get a good understanding of how StringIO/ARGF works by reading about StringIO/ARGF.

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