music-encoding / sibmei

Sibelius MEI Plugin
MIT License
40 stars 16 forks source link

Text handling improvements #173

Closed th-we closed 3 years ago

th-we commented 3 years ago

Main differences:

Indentation

If there is text content, libmei won't intersperse it with formatting white space any more. This is especially important for the new text formatting features. If we have original text without whitespace in between, we don't want this:

<p>
    <rend fontweight="bold">
        foo
    </rend>
    :
    <rend fontstyle="italic">
        bar
    </rend>
</p>

We do want this:

<p><rend fontweight="bold">foo</rend>:<rend fontstyle="italic">bar</rend></p>

Formatting tracking

The logic for tracking how the rendering state changes while traversing a formatted string is less twisted and more robust now.

ahankinson commented 3 years ago

@rettinghaus please undo this -- it contains changes that have been 'leaked' in from another PR, that I'm not quite ready to approve.

rettinghaus commented 3 years ago

Sorry I messed something up here.

rettinghaus commented 3 years ago

@th-we please open a new PR for this.

annplaksin commented 3 years ago

Sorry, I overlooked the changes in the libmei as they were to many to be displayed in the diff. Since PRs cannot be reopned, a new PR would be the best idea... the improvements of the text handling are useful though.

ahankinson commented 3 years ago

Agreed -- the text improvements should be kept on a separate branch.

ahankinson commented 3 years ago

In fact, in general I would suggest making only changes that need to be made for a certain bit of functionality. I know the temptation is high to "just do this one thing", but it makes it really hard for reviewers to know whether a change is intentional or will cause problems. (Like this PR changing the tuplet._parent = null; line -- not sure why that needs to be changed in a PR that is for text handling).

th-we commented 3 years ago

@ahankinson The tuplet._parent = null removal is because in that commit, that functionality was moved to libmei. I'll create more granular pull requests, leaving out the XML parser removal.