musescore / musescore/MuseScore
Refactor TextBase and text editing
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 15.1k
- Forks
- 3.3k
- Avg merge
- 2d 2h
- Merged PRs (30d)
- 91
Description
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, firstTextBase::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: thexmlText, 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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading the TextBase and TextCursor concepts described here, then trace their interaction with NotationInteraction and the related pull requests. The issue is currently a planning discussion rather than a bounded implementation task; done would require an agreed design and clearly defined follow-up scope.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- desktop
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 15/100