Added TextEditMenu for handling menu for TextEdit and LineEdit

[TheSofox]

Fixes #85511

This PR creates a new class TextEditMenu which contains code that was previously duplicated in TextEdit and LineEdit. Specifically, it's for handling the context menu that pops up when you (for example) right click on a text field.

[AThousandShips]

You are removing the bound enum values, this breaks compatibility, they should be left as they are and extracted from the menu instead

So simply copy the enum into this by:

enum MenuItems {
		MENU_CUT = TextEditMenu::MENU_CUT,
...
};

And then you don't need to rename them here, this is done elsewhere as well

[KoBeWi]

So simply copy the enum into this by:

Also they should be marked as deprecated.

[AThousandShips]

Assuming we don't want them available any more, unsure if we do but without that we can't control the menu options by code

Note that the menu item helper class isn't exposed so we can't access these things through that

[KoBeWi]

Right, they should be available somehow… 🤔
idk where to put the enums then. TextEditMenu would best stay internal. Maybe it could use TextEdit's enums, but that sounds meh.

[AThousandShips]

I'd keep them separate for now, and just copy the values from the menu

We could make it a global enum as well, assuming its general enough

[TheSofox]

Restored enums into TextEdit and LineEdit, including their bindings, and added the copying of their values from TextEditMenu as suggested by @AThousandShips.

[AThousandShips]

Haven't tested but looks good to me, will look further when I have the time

[KoBeWi]

Looks fine overall.

[Paulb23]

I don't entirely agree with this change, code can be too DRY and we need to be careful of that. As it introduces extra abstractions and hidden coupling.

This goes to #85624 as well.

As long as these two classes have existed there has always been a discussion about combining them in some way. Although they certainly look similar on the surface they do serve completely different use cases (Along with RichTextLabel).

Before starting to make any changes in this direction, I'd suggest creating a detailed proposal of the direction / end goal here.

[TheSofox]

Why in the world did you not say this when the Pull Request first went live? We've spent 4 days tweaking, refining, fixing and discussing this. I could have put my time and energy into other things. Pretty demotivating.

[AThousandShips]

You can't expect someone to notice immediately and be around, everyone has their own schedule and might not notice, it's not Paulb23's fault

[TheSofox]

When I posted this PR, I expected it to be evaluated on a conceptual level before we moved onto "discuss, fix and tweak a lot of various parts of it." There's over 10 reviews I had to respond to, for instance, all of which turned out to be pointless because the PR wasn't going to be accepted no matter how many tweaks I made to it.

I'm not saying its Paul's or anybody's fault, I recognise you all have your things to focus on, but meanwhile I followed a process that led to me needlessly wasting time and effort in a way that seemed totally avoidable.

[AThousandShips]

Well the people who reviewed this did (and still do in my case) consider this a worthwhile change, there were no direct questions on that side, also this wasn't really tweaks but getting the basic functionality up, not polishing

There's no set decision here, so nothing is necessarily wasted, and it wasn't really avoidable since no concern like this was raised before the work was done. I don't consider my time wasted on my part (despite the work put in), but I understand your frustration.

However that's the reality of development, and the points on it having been discussed in the past was mentioned already by KoBeWi, and at least I didn't see any issue with this approach (I did with the previous approach without it being more complete), and I assumed you were considering the possiblity of this approach not being accepted, and I assumed that KoBeWi didn't either given the feedback on this.

[TheSofox]

I understand the points you're making, and thank you for recognising my frustration.

I recognise that the contributions I made on this branch and the other branch were "proof of concept" to see whether the idea worked, and both PRs can be looked at for their approaches from now on, whether or not their code is ultimately used.

In terms of future, it looks like it can't proceed until the "detailed proposal" is written and the next thing to decide is who is going to write it.

[KoBeWi]

Eh, I opened the issue after I noticed the code duplication while working on #85477. It was meant for discussing the solution, but after you opened a PR I just proceeded to review it normally, sorry 🙃

I still think extracting the menu has its merits and IMO could be done without a proposal. I never suggested merging TextEdit and LineEdit.

[TheSofox]

Hey, after having some time to cool down, I recognise that while I was raising a genuine concern, I may not have expressed it in the most professional way. I also took Paul's comment as a flat rejection rather than "more work needed" which I apologise for.

I do agree that reducing repetition between TextEdit and LineEdit is a good idea. A quick diff compare of the two header files is very instructive. However, various solutions have their own complexities.