Add missing documentation to String methods

[jan-zajic]

n/c

[jan-zajic]

Hi @Sija, @asterite, @straight-shoota, @r00ster91, second round. Also i fix bug that i found during creating code samples in byte_slice(Int): currently it raises Negative count (ArgumentError) for large index (for example "hello".byte_slice(6)), which makes no sense. It should raise IndexError.

[straight-shoota]

Could you please squash the changes into two commits: One fixing the exception, the other adding the docs?

[jan-zajic]

@straight-shoota yes, of course.

[straight-shoota]

1. To avoid confusion, I'd suggest to always specify "byte index" to distinguish from character index. Maybe we should also use "character index" in the other cases as well, but I'm not sure.
   Perhaps even rename the arguments to byte_index or byte_offset? I'm not sure that's necessary, though. And it should be a different PR.

2. You're sometimes referring to "out of bounds", sometimes "out of range". I'm not sure if one is strictly better than the other but it would be best to use the same terminology everywhere.

3. Regarding: "Returns true, if [...], otherwise false.": IMO the last part should be omitted. It's unnecessary and only clutters the description. It should be evident that Bool methods return false in all other cases where it's not true.

4. Probably also a thing for another PR, but I'd like to standardize some of the argument names. For example byte_slice(start : Int, count : Int) vs. unsafe_byte_slice(byte_offset, count) (see also 1.). And short argument names like str or re should IMO be properly named.

[jkthorne]

@jan-zajic thank you for all your work on docs!

[jan-zajic]

@straight-shoota i made fixes according to yours review + according to your comment:
ad 2, it's almost 50:50 in current String doc. All replace range to bounds in whole file..
ad 3, I agree with you, done.
1, 4, good point but left for another PR, as we already have a lot here.

[straight-shoota]

Thank you very much!

[straight-shoota]

@jan-zajic Could you please rebase on master?

[jan-zajic]

@straight-shoota yes, of course. It's done.

[RX14]

"Returns the index of the first ocurrence"?

[jan-zajic]

@straight-shoota what you think about this proposal? Is it not against previous considerations? I think my draft is in line with other docs in string file..

[straight-shoota]

I'm not sure, did we discuss this before? @RX14's suggestion sounds reasonable, it's more specific.

[jan-zajic]

Maybe, but it's same with current existing documentation for index method .. So maybe i have to change it too? And please write always complete formulation, what about for example:

1, Returns byte index of the first ocurrence of byte in the string, or nil if not present.

or

2, Returns the index of the first ocurrence of byte in the string, or nil if not present.

or

3, Returns the index of the first ocurrence in the string, or nil if not present.

or

4, Returns byte index of the first ocurrence in the string, or nil if not present.

or?

[straight-shoota]

Number 2 please.
And I'd like to change #index accordingly, that is add "first occurrence" to the existing phrase. For the regex overload: "first match"

[RX14]

It seems this change was lost.

[jan-zajic]

@straight-shoota i rebase this on current master (again). Please can we finally merge this, before another conflicts arise?

[RX14]

Just one thing got lost in the rebase I think.

[RX14]

It seems this change was lost.

[jan-zajic]

@RX14 it's done