Spectrogram context menu and spectral-delete shortcut#10465
Merged
saintmatthieu merged 33 commits intoaudacity:masterfrom Mar 17, 2026
Merged
Spectrogram context menu and spectral-delete shortcut#10465saintmatthieu merged 33 commits intoaudacity:masterfrom
saintmatthieu merged 33 commits intoaudacity:masterfrom
Conversation
94a4d27 to
b9ff86c
Compare
49052de to
f0d5ff5
Compare
luapmartin
requested changes
Mar 6, 2026
src/spectrogram/qml/Audacity/Spectrogram/TrackSpectralSelectionContainer.qml
Show resolved
Hide resolved
bbe7de8 to
cb0a085
Compare
luapmartin
reviewed
Mar 11, 2026
luapmartin
reviewed
Mar 11, 2026
src/uicomponents/qml/Audacity/UiComponents/components/MarqueeSelection.qml
Show resolved
Hide resolved
luapmartin
reviewed
Mar 12, 2026
cb0a085 to
10575ee
Compare
luapmartin
reviewed
Mar 13, 2026
Comment on lines
+1339
to
1352
| const auto clipsAfter = SortedIntervalArray(); | ||
| // If clips before and after have the same play start and end times, apply the IDs of the original clips: | ||
| if (clipsBefore.size() == clipsAfter.size()) { | ||
| for (size_t i = 0; i < clipsBefore.size(); ++i) { | ||
| if (clipsBefore[i]->GetPlayStartTime() != clipsAfter[i]->GetPlayStartTime() | ||
| || clipsBefore[i]->GetPlayEndTime() != clipsAfter[i]->GetPlayEndTime()) { | ||
| return; | ||
| } | ||
| } | ||
| for (size_t i = 0; i < clipsBefore.size(); ++i) { | ||
| clipsAfter[i]->SetId(clipsBefore[i]->GetId()); | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
I guess it's ok with undo/redo right?
Contributor
Author
There was a problem hiding this comment.
I think so, yes.
luapmartin
approved these changes
Mar 13, 2026
Contributor
luapmartin
left a comment
There was a problem hiding this comment.
Approved, the last question would be that with such a dense feature and lot of little know-how here and there, would proper unit testing make sense to avoid (control) regressions?
I guess there is low hanging fruits in terms of basic operation that would cover a wide range of cases that QA wont have to venture on testing each and every time.
Contributor
|
Issues outlined in the QA checklist above have been fixed. |
10575ee to
0716864
Compare
Instead, disable spectral effect actions when they would fail anyway.
0716864 to
ff63b81
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves: Spectral editing commands #10190
Resolves: Spectrogram button in the main toolbar doesn't change its state to active when pressed #10445
Resolves: The cursor doesn't change when hovering over the marquee selection when in Spectrogram mode #10476
Resolves: Scrolling the track list up and down causes one of the track's spectrogram go blank #10393
Resolves: Don't rely on out-of-process plugin scanning for nyquist plugins #10480
Resolves: Nyquist Effects Copy Audio Data when applied to Clip Selection #10491
Could resolve: Applying Graphic EQ to a range selection creates a new clip #10517
Recommended:
QA: