Fix for #1907 - inconsistent clamping behavior in automation editor #1910
Conversation
| @@ -699,12 +694,12 @@ void AutomationEditor::mouseMoveEvent(QMouseEvent * mouseEvent ) | |||
| { | |||
| if( QApplication::overrideCursor() ) | |||
| { | |||
| if( QApplication::overrideCursor()->shape() != Qt::SizeAllCursor ) | |||
| if( QApplication::overrideCursor()->shape() != Qt::SizeAllCursor ) | |||
There was a problem hiding this comment.
Would you mind to replace spaces with tabs? This seems to be convention on this project.
|
This looks good. Very small change too, which is nice! If @badosu or @curlymorphic have a chance to review this, please do. If not, it will be a while before I test this fix out myself. |
|
Scratch that, @badosu is already reviewing this. 👍 |
|
@tresf It looks like the relevant change consist of only the 3 lines that were removed. A more thorough test is necessary to ensure that the removed check was not important for other code paths. I'll take a look later. |
|
Oops! I forgot my editor was configured for spaces. Will revise that later today. |
|
@Wallacoloo On your PR's description you refer to line 894 but you didn't change it, is this intended? |
|
@badosu IIRC, he was talking about the lines which had redundant logic to what the removed lines did (I'd guess as a sanity reminder that he wasn't going to send negative values into the code). |
|
@tresf correct.
|
|
@Wallacoloo I got what you mean now, good job! 👍 Gonna test again as soon as you push the style fix. |
|
@badosu All the code touched in this pull request shows up as using tabs for indentation on both Github and my editor (Sublime Text). Can you clarify what you mean about replacing the spaces with tabs? |
|
@Wallacoloo If this is the case I am deeply sorry, I guess I am used to represent tabs in a different manner on my editor. I'll test this PR tomorrow, thanks! |
|
It works! 👍 |
|
@badosu, much obliged. |
Fix for #1907 - inconsistent clamping behavior in automation editor
The safety check for x < 0 at the topmost level was mostly redundant. The check to make sure we don't move an automation point to some t < 0 is handled on line 640 instead, as well as line 894 for the case where m_editMode == SELECT.
This slightly changes behavior for deleting automation points as well. Previously, it had a similar inconsistency: you could delete points off to the right of the view, but not off to the left. Now you can delete points from both the left and right side.
I also fixed some inconsistent tab formatting.
#1907