cbjeukendrup
commented
2 hours ago My proposals
-
Introduce a
TextEditingSessionclass, that lives in NotationInteraction land. All text interaction logic should be moved to this class. When the user double-clicks a text element, a TextEditingSession object would be created, rather than any kind of edit mode activated. When text editing is exited, the TextEditingSession is reset / deleted. When receiving a key press event, NotationInteraction would check if there is a TextEditingSession active. If yes, give the TextEditingSession the chance to handle this event. If it doesn't handle the event, do something else with it. When painting, NotationInteraction would also check if there is a TextEditingSession; if yes, let the TextEditingSession do some painting too, on top of the score. A stretch goal would be to even get rid ofTextBase::drawEditMode. -
Make
layoutData->blocksthe single source of truth for text content. This means that it probably needs to be lifted out oflayoutDataand made a member of TextBase. However, layout-related data should stay inlayoutData, such as dimensions and positions. So perhaps the current TextBlock etc needs to be separated into a graphical part that stays inlayoutDataand a logical "plain old data" part that goes to TextBase. -
Reimplement the TextCursor class. The idea of this class as a way to manipulate text seems not bad at all. Only the current implementation is bad and incomplete. A more general and versatile implementation should be made, ideally one that can be used both to underpin the UI editing interaction and to construct/manipulate text programmatically.
These are admittedly relatively high-level ideas, and not yet elaborated very much.
We've already reached consensus that this is necessary; just creating an issue so that we can plan it, and discuss how it should be done.
The problems
Currently, a lot of text editing code is inside the TextBase class and its subclasses; this means in fact that UI interaction logic is mixed with "business logic". And some things have to be done outside TextBase, so happen in NotationInteraction, so the logic is spread over various places. This is also observed in https://github.com/musescore/MuseScore/pull/24693 (at least at the time of writing): to allow navigating between text objects, first
TextBase::isEditAllowedneeds to be overridden so that the necessary keyboard shortcut is not swallowed by the TextBase, and then the actual logic needs to be implemented in NotationInteraction. This creates a tight but not-apparent relation betweenisEditAllowedoverrides and NotationInteraction.A more general problem on the border between NotationInteraction and the engraving module, is the legacy "edit more". Very different kinds of editing functionality are currently based on it; basically, element dragging, grip dragging, and text editing. The latter two cause problems for dynamics, which should support both; see for example https://github.com/musescore/MuseScore/pull/25215 and related PRs. The full solution would be to factor out the concept of a general edit mode, and implement specific logic for different kinds of editing, but we could start with decoupling text editing from this general edit mode.
Manipulating TextBase objects programmatically is cumbersome, especially when formatting is involved, as @mathesoncalum experienced with https://github.com/musescore/MuseScore/pull/23475 for example. It is unclear how that is supposed to be done, since there are so many ways and all of them seem to solve only half of the problem. There are sort of three sources of truth: the
xmlText, thelayoutData->blocks, and theplainText. Of these, onlylayoutData->blocksseems usable, but even that feels like a hack, because normally those blocks are calculated during layout. There is also theTextCursorclass. That sounds like a convenient way of manipulating the text object, but in reality its set of methods is very limited and UI oriented, so it's not really usable either.Solutions Let's discuss possible solutions below.