Add TextControl to unify code of TextEdit and LineEdit

[TheSofox]

Fixes #85511

Added TextControl as the parent of TextEdit and TextLine to consolidate code all the code between them, starting with the parts in listed in #85511

There's a lot more code that could be consolidated, but started with the sections listed in the bug report, just to test whether this is a viable solution.

[AThousandShips]

I'm not sure about the usefulness of changing the inheritance for a single method, which as far as I know breaks compatibility as it rearranges the inheritance, but more importantly moves the enum which breaks compatibility as well

I think a less disruptive change is warranted here unless more shared code is unified

Further the new class should be abstract as you shouldn't be able to instance it as it's incomplete

[TheSofox]

I'm not sure about the usefulness of changing the inheritance for a single method,

There are other methods, such as menu_option(int p_option), various selection methods (eg is_selecting_enabled()). and a lot of other methods/variables. where similar things are done but worded differently. Reducing this repetition would need to be an ongoing process with a bit being done at a time.

but more importantly moves the enum which breaks compatibility as well

Conceptually, the bug requires this break if it's going to be fixed. The entire issue is that there are two identical enums in separate classes. Either one is going to be removed or both are going to be merged in a separate place. Either way, at least one enum gets moved.

I think a less disruptive change is warranted here unless more shared code is unified

I'm open to suggestions.

Further the new class should be abstract as you shouldn't be able to instance it as it's incomplete

Makes sense.

[KoBeWi]

tbh this is the least breaking change regarding the enums, because you can still use them directly in both classes and they retain their values.

[AThousandShips]

Reducing this repetition would need to be an ongoing process with a bit being done at a time.

I disagree, doing this large a change in the hierarchy for a single method and then possibly doing more in the future doesn't feel constructive to me

It presumes that the change is needed in this form without adding all the shared code, if all the shared code is added in one PR that change makes sense, but otherwise it's assuming we need this compatibility breakage here instead of doing it some other way yet locking us into this change

[AThousandShips]

You need to move registering the enum to this class, with _bind_methods and extracting the binding from the two other classes

The binding would be strange otherwise as they are two different enums (especially in C#)

[TheSofox]

It presumes that the change is needed in this form without adding all the shared code, if all the shared code is added in one PR that change makes sense, but otherwise it's assuming we need this compatibility breakage here instead of doing it some other way yet locking us into this change

I get what you're saying. You want the PR to do it all in one go so it can be evaluated on its own merits.

I'm gonna have to give this more thought. In the meantime I don't mind if anyone else wants to try their own approach or work off of what I've done.

[AThousandShips]

For the menu one option would be to create a dedicated popup menu to share between these two, and provide it a method to call, similar to how ViewPanner works

[TheSofox]

That's actually the approach I'm looking into right now, I'm glad you think it would be a good solution to. Also thanks for mentioning ViewPanner, I didn't know about it before and it'll be very useful having a comparison.

[YuriSizov]

The linked issue already covers the question of whether it would be rational to try and unify these controls. So I think going into this direction would be counter-productive. At the very least, it's a controversial and contested idea, and so doing a breaking change over it would be hard to justify.

There are shared bits of code, and I believe most of that duplication can be reduced without affecting user-facing facilities. I would suggest efforts to be put towards this solution instead.

[TheSofox]

In response to the feedback I've received, I've created an new PR that solves this issues through creating a separate class that handles the Context Menu: #85720

I believe that PR solves the issue in a far more focused way with far more compatibility preserved, and is therefore a better solution.

[MewPurPur]

I've noticed that LineEdit does a lot of things poorly compared to TextEdit and thought a lot about this. These two classes are very different in everything aside from text editing, which encompasses basic things like text selections, drawing carets, undo/redo, and others.

I'm not sure if it's a good or a bad idea. On one hand, it would make LineEdit a wrapper around something way more complicated than necessary for it (caret positions being 2D vectors, multi-caret). On the other hand, it would ensure that there are no differences between the nodes' behaviors (like the new ones from #86978).

All things considered, I think it's a bad idea.