Crystal: String#tr with empty String

[AlexWayfer]

Hello!

"aabbcc".tr("b", "")

Output:

Index out of bounds (IndexError)
[4311543] *CallStack::unwind:Array(Pointer(Void)) +87
[4311434] *CallStack#initialize:Array(Pointer(Void)) +10
[4311386] *CallStack::new:CallStack +42
[4295912] *raise<IndexError>:NoReturn +24
[4397942] *Char::Reader#next_char:Char +38
[4338141] *String#tr<String, String>:String +14253
[4305298] *__icr_exec__:String +130
[4292040] ???
[4304873] main +41
[140625133621969] __libc_start_main +241
[4290714] _start +42
[0] ???

In Ruby:

"aacc"

It's not cool :(

[jhass]

It should at least give a better error message, but I'm actually not sure about special case. You can just use String#delete for it.

[AlexWayfer]

@jhass Okay, thanks, I will use String#delete now.

[masukomi]

@paddor I'm not understanding why throwing an ArgumentError is the correct solution to this. What's wrong with replacing something with an empty String from a conceptual / usability standpoint? I think this solution violates the Principle of Least Surprise. I would absolutely expect it to work like @AlexWayfer did.

I like that if the exception is going to be thrown it says "to can't be empty" and that does help, but still...

[masukomi]

also, if this issue is no longer up for debate because of the "don't crash if replacement is empty" commit, it should be closed. If it _is_ up for debate though, let's debate! ;)

[paddor]

@masukomi Maybe you wanna read up on the discussion on #3129.

[AlexWayfer]

3129 is merged, behavior is expected, issue can be closed?