Add String#index/rindex! methods

[Sija]

Adds String#index! and String#index! methods, similar to the ones found in Enumerable.

Supersedes/closes #12729

[beta-ziliani]

Just an observation. Thanks Sija! <3

[asterite]

IndexError doesn't make sense here. Index error means the index was out of bounds. That's not the case here.

[Sija]

@asterite Any ideas for a better ones?

[asterite]

Enumerable::NotFoundError

[Sija]

@asterite I have a bit of an issue with this, since String is not an Enumerable, technically speaking.

[asterite]

Then NotFoundError should be moved to the top level.

[asterite]

Just like IndexError, KeyError, etc., are all at the top level. We can probably keep the ones in Enumerable as an alias of the top level ones.

Just my opinion though! I'm sure others will propose more complex solutions.

[Sija]

@asterite NotFoundError in the top-level is gonna be IMO too generic, or maybe not...?

[straight-shoota]

Let's just use Enumerable::NotFoundError here as we do in Enumerable#index!.
Move that type to the top-level namespace should be a separate discussion.

[asterite]

Why NotFoundError would be too generic, but IndexError or KeyError would not? Maybe it could be called ElementNotFoundElement or something like that.

Another idea: just use whatever .not_nil! does for now.

[Sija]

@beta-ziliani Good catch, fixed!

[sdogruyol]

Thank you @Sija 🙏

[HertzDevil]

#rindex! is uncalled for; #11566 didn't add it to the standard library and Ameba does not warn on it. It would be odd to have it here before the collection types do.

[straight-shoota]

IMO #rindex! makes as much sense as #index!, so there would be no harm in adding it. We should add it to Enumerable in the same manner.

[HertzDevil]

I fully agree with adding #rindex!, I just think it should be organized into:

• String#index!
• #rindex! on everything

rather than:

• String#index! + String#rindex!
• #rindex! on everything else

[straight-shoota]

Oh, okay. "#rindex! is uncalled for;" reads like you wouldn't approve of it whatsoever.

IMO the organizational aspects don't matter much. String#index! and String#rindex! are related and we have a ready-to-go PR for them. I see little point in moving things around into different PRs.