Range selection ending fix #933
Conversation
| @@ -280,6 +280,10 @@ void Selection::updateSelectedElements() | |||
| for (Segment* s = _startSegment; s && (s != _endSegment); s = s->next1MM()) { | |||
| if (s->segmentType() == Segment::SegEndBarLine) // do not select end bar line | |||
| continue; | |||
| if (s->segmentType() == Segment::SegTimeSig) { | |||
| Segment* next = s->next1MM(); | |||
| if(!next || next == _endSegment) continue; | |||
There was a problem hiding this comment.
Maybe a separate line for "continue" could be better for code readability?
|
I am not sure if this is the best way to fix the issue. What I have done is added a check in the drawing methods to stop drawing when the current segment is a end barline or a time signature and they are at the end of the selection. The barline and time signature should not be there to begin with. I am looking now at a way to do this. Closing this issue for now. |
|
This change will make it impossible to select a breath segment. How should we fix this? Maybe allow the user to extend the range selection onto these breath segments by clicking on them? |
|
The current change treats a breath as a part of the previous CR. This way we don't run into the problem of selecting a breath. |
BartlomiejLewandowski
changed the title
| @@ -1144,5 +1144,16 @@ void ChordRest::writeBeam(Xml& xml) | |||
| #endif | |||
| } | |||
|
|
|||
| Segment* ChordRest::nextSegmentAfterCR(Segment::Type types) const | |||
There was a problem hiding this comment.
Please add a comment before, see all other functions in this file. Explain what the function is really doing. Especially the (s->tick() >= tick() + actualTicks())
Also it needs space after while, after if etc...
Last thing... I'm not a big fan of the while (true), and multiple return in the function.
What about something like this?
Segment* ss = segment();
for (Segment* s = ss; s && (s->tick() < tick() + actualTicks());) {
s = s->next1MM(types);
}
return s;
There was a problem hiding this comment.
The Segment* s is undefined in your snippet at return s.
I don't know if this much clearer. It is a bit more compact. If we are going this way, why not move the loop body to the last part of the for loop? We would remove the need of the body fully.
Segment* s = segment()->next1MM(types);
for (s; s && (s->tick() < tick() + actualTicks()); s = s->next1MM(types));
return s;
There was a problem hiding this comment.
I was just trying to avoid the while true. The compactness is not really the goal. I would be ok with putting the tick test in the loop if you think it's clearer.
|
I have rewritten the |
Removed time signature from selection if not followed by chordrest in selection.
Fixed range box to not include bar lines and time signatures if not followed by a chordrest.