What is the sense of String.erase( int position, int chars )?

[VardenTheOne]

Godot version:
Any

OS/device including version:
Any

Issue description:
What is the sense of String.erase( int position, int chars ) if it does not return anything?
https://docs.godotengine.org/en/stable/classes/class_string.html#class-string-method-erase

[MWFIAE]

It changes the string :)

Seems like string in gdscript are indeed mutable ;)
PS: It is however weird as the documentation states that strings are supposed to be copy-on-write and all other functions do respect this convention

[KoBeWi]

Looks like this is the only String method that modifies the String instead of returning a new one 🤔

[Calinou]

I'd vote for revising this method to make it immutable like other String methods. What should we rename it to?

[VardenTheOne]

Yeah, this is confusing for me. It is better when all functionality works the same way.

[MWFIAE]

I'd vote for revising this method to make it immutable like other String methods. What should we rename it to?

Maybe something like: splice?

[Calinou]

@MWFIAE Sounds good to me. The method would behave roughly like JavaScript's Array.splice() then.

[theoway]

In Javascript, splice() changes the original array whereas slice() doesn't but both of them return array object. So, maybe we should go for something like slice() to maintain immutability of String.

[Calinou]

@theoway We already have Array.slice() which has different behavior, I fear giving a different behavior to String.slice() would make it confusing.

[theoway]

@theoway We already have Array.slice() which has different behavior, I fear giving a different behavior to String.slice() would make it confusing.

Indeed, it'd be confusing. Then it'd be a good option to implement something like splice(like in Javascript)while maintaining string immutability.

[denormative]

Indeed, it'd be confusing. Then it'd be a good option to implement something like splice(like in Javascript)while maintaining string immutability.

Why not just update erase to return the new string and not mutate the string as a backwards incompatible break for 4.0? Since going forward you'd need to do something with 'erase' anyway. It seems cleaner then cluttering the API up with another method name.

[theoway]

Indeed, it'd be confusing. Then it'd be a good option to implement something like splice(like in Javascript)while maintaining string immutability.

Why not just update erase to return the new string and not mutate the string as a backwards incompatible break for 4.0? Since going forward you'd need to do something with 'erase' anyway. It seems cleaner then cluttering the API up with another method name.

Makes sense. But the method name needs to be changed, as it won't be erasing anymore(after changing the method)

[Xrayez]

Upon thinking beyond the method itself, we have a similar method String.insert(), which is immutuable and returns a new String...

as it won't be erasing anymore

Neither insert would insert a new substring into existing one, so perhaps it also has to be renamed? Not sure... 🤔

[theoway]

Neither insert would insert a new substring into existing one, so perhaps it also has to be renamed? Not sure... 🤔

Indeed. If anyone has suggestions as to what it should be renamed to, drop 'em here. I'll make a PR to change the erase() to something like splice and change the name of insert()

[Calinou]

Maybe we should remove/unexpose String.erase() entirely in 4.0? Mutable string methods can cause more trouble than they solve.

[theoway]

Maybe we should remove/unexpose String.erase() entirely in 4.0? Mutable string methods can cause more trouble than they solve.

That's a good idea. It can be replaced with a similar method, like splice(in Javascript), making sure that it doesn't mutate the string. How does that sound?

[Calinou]

@theoway Isn't JavaScript's Array.splice() the same as Array.slice() but in a mutable form? Going by this question on Stack Overflow, I can't see a difference other than the mutability.

[theoway]

@theoway Isn't JavaScript's Array.splice() the same as Array.slice() but in a mutable form? Going by this question on Stack Overflow, I can't see a difference other than the mutability.

Also, splice can add elements at the index from where old elements were removed.
So, in a way, it can do the job of insert() & erase()
See this article: https://www.javascripttutorial.net/javascript-array-splice/

[Calinou]

@theoway In this case, I guess it makes sense to implement an immutable version of String.splice().

[theoway]

@theoway In this case, I guess it makes sense to implement an immutable version of String.splice().

Should I make the PR of implementation or make a proposal first?

[Calinou]

@theoway I think you can open a pull request directly against the master branch and remove String.erase() at the same time.

[theoway]

@Calinou I had one more question. Since, we're implementing splice, it'll make insert() redundant in gdscript string. I was wondering if we should get rid of it as well.

[Calinou]

@theoway I guess it makes sense to remove String.insert() as well in this case.

[theoway]

@Calinou Now there are many references to erase() and insert() throughout the project. So, should I remove these methods and make changes everywhere or add the new method(i.e. splice()) while keeping these two(erase() and insert()), but removing their references from engine's scripting languages(gdscript, gdnative etc.)?

[golddotasksquestions]

I am currently trying to test RichTextLabel nodes in 4.0 master because the new RTL got a whole bunch of features I really need in my project. My project is very heavy in dynamic text. Which means I need to format BBCode text of the RTL a lot, which in turn means I need to erase and insert (not necessarily replace) a lot of strings.

Right now, I can't really do any of my testing because the latest master build appears to have erase() removed, but splice does not exist either.

Also, just to note, erase is still featured in the online API documentation, however not in the build in API documentation. splice() is mentioned nowhere. The current master also gives me a showstopping error saying "this property does not exist".

I would really love to test the new RTL BBCode features, but without being able to edit strings properly, there really is no point. It would be really nice if someone could reinstate erase() until we actually have splice() or any better solution.

[Calinou]

Right now, I can't really do any of my testing because the latest master build appears to have erase() removed, but splice does not exist either.

Also, just to note, erase is still featured in the online API documentation, however not in the build in API documentation. splice() is mentioned nowhere. The current master also gives me a showstopping error saying "this property does not exist".

The erase() method isn't exposed in master since it mutates the String. The new core binding system made it difficult or impossible to expose some methods, as explained in #43311.

Closing in favor of #43311, since we'll have to see whether we can actually expose a String.erase() method for 4.0.