-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Fix "select similar in range" for spanners #27577
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix "select similar in range" for spanners #27577
Conversation
Nowadays, not Spanners, but SpannerSegments are included in `m_selection.elements()`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved - included a couple of small observations.
if (p->measure) { | ||
auto eMeasure = e->findMeasure(); | ||
if (!eMeasure && e->isSpannerSegment()) { | ||
if (auto ss = toSpannerSegment(e)) { | ||
if (auto s = ss->spanner()) { | ||
if (auto se = s->startElement()) { | ||
if (auto mse = se->findMeasure()) { | ||
eMeasure = mse; | ||
} | ||
if (auto s = toSpannerSegment(e)->spanner()) { | ||
if (auto se = s->startElement()) { | ||
if (auto mse = se->findMeasure()) { | ||
eMeasure = mse; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we're at it, we ideally reserve auto
for iterators and other complex types right? I think it's unnecessary here.
Also as a bonus I'd love to flatten this nesting a little - perhaps by checking !p-measure
and returning early? (maybe that ends up even uglier, its up to you).
Tested #27548 on Win10, Mac13.7.2, LinuxUbuntu24.04.2 LTS - FIXED
Yes, they were broken too. Now work fine |
Nowadays, not Spanners, but SpannerSegments are included in
m_selection.elements()
.Resolves: #27548
QA note: seems the issue applied to other spanner elements as well (hairpins, text lines, ...); would be good to test those too.