musescore / musescore/MuseScore

Refactor TextBase and text editing

Open
#25,229 3 comments 3 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

tech debt
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, first TextBase::isEditAllowed needs 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 between isEditAllowed overrides 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, the layoutData->blocks, and the plainText. Of these, only layoutData->blocks seems usable, but even that feels like a hack, because normally those blocks are calculated during layout.
    There is also the TextCursor class. 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.