refactor rename-uis, allow naming CV clips as well by nikodemus · Pull Request #3183 · SynthstromAudible/DelugeFirmware

nikodemus · 2024-12-30T20:15:42Z

allow deleting names (by replacing them with empty) for all but drums
consolidate shared code in RenameUI class: subclasses implement only a couple of methods to specialize their behaviour
ad-hoc dispatch in sound_editor.cpp replaced with normal shortcut machinery
shrinks the binary by just a hair, but shrinks, not grows!

github-actions · 2024-12-30T20:18:27Z

Test Results

106 tests ±0 106 ✅ ±0 0s ⏱️ ±0s
16 suites ±0 0 💤 ±0
16 files ±0 0 ❌ ±0

Results for commit 863eaf8. ± Comparison against base commit 89720ba.

♻️ This comment has been updated with latest results.

sapphire-arches · 2025-01-02T03:45:44Z

LGTM, though there's now a merge conflict.

- allow deleting names (by replacing them with empty) for all but drums - consolidate shared code in RenameUI class: subclasses implement only a couple of methods to specialize their behaviour - ad-hoc dispatch in sound_editor.cpp replaced with normal shortcut machinery - shrinks the binary by just a hair, but *shrinks*, not grows!

Co-authored-by: Sapphire Koser <srkoser+GitHub@gmail.com>

nikodemus · 2025-01-02T10:01:56Z

Updated, and found the same title_ -> title bug twice more. I've no idea how those passed my own testing... 😅

This reverts commit 0d55ca5.

* Revert "refactor rename-uis, allow naming CV clips as well (#3183)" This reverts commit 0d55ca5. * Bugfix for rename-UIs PR

…#3183) * refactor rename-uis, allow naming CV clips as well - allow deleting names (by replacing them with empty) for all but drums - consolidate shared code in RenameUI class: subclasses implement only a couple of methods to specialize their behaviour - ad-hoc dispatch in sound_editor.cpp replaced with normal shortcut machinery - shrinks the binary by just a hair, but *shrinks*, not grows! * fix initialization of RenameUIs Co-authored-by: Sapphire Koser <srkoser+GitHub@gmail.com> --------- Co-authored-by: Sapphire Koser <srkoser+GitHub@gmail.com>

* Revert "refactor rename-uis, allow naming CV clips as well (SynthstromAudible#3183)" This reverts commit 0d55ca5. * Bugfix for rename-UIs PR

refactor rename-uis, allow naming CV clips as well (SynthstromAudible#3183) * refactor rename-uis, allow naming CV clips as well - allow deleting names (by replacing them with empty) for all but drums - consolidate shared code in RenameUI class: subclasses implement only a couple of methods to specialize their behaviour - ad-hoc dispatch in sound_editor.cpp replaced with normal shortcut machinery - shrinks the binary by just a hair, but *shrinks*, not grows! * fix initialization of RenameUIs --------- Co-authored-by: Sapphire Koser <srkoser+GitHub@gmail.com> Fix rename-UIs PR (SynthstromAudible#3218) * Revert "refactor rename-uis, allow naming CV clips as well (SynthstromAudible#3183)" This reverts commit 0d55ca5. * Bugfix for rename-UIs PR add section number to clip names, copy clip names along with clips (SynthstromAudible#3195) * use section numbers with clip names - if clip has no name, display "Section: <section num>" - if clip has a name, display "<section num>: <clip name>" * copy clip names when duplicating clips - append a number to the clip name if necessary to avoid duplicates on the same output; if the name already ends with a number, the existing number is incremented instead Co-Authored-By: Nikodemus Siivola <nikodemus@random-state.net> Co-Authored-By: Katherine Whitlock <122173759+stellar-aria@users.noreply.github.com>

refactor rename-uis, allow naming CV clips as well (SynthstromAudible#3183) * refactor rename-uis, allow naming CV clips as well - allow deleting names (by replacing them with empty) for all but drums - consolidate shared code in RenameUI class: subclasses implement only a couple of methods to specialize their behaviour - ad-hoc dispatch in sound_editor.cpp replaced with normal shortcut machinery - shrinks the binary by just a hair, but *shrinks*, not grows! * fix initialization of RenameUIs --------- Co-authored-by: Sapphire Koser <srkoser+GitHub@gmail.com> Fix rename-UIs PR (SynthstromAudible#3218) * Revert "refactor rename-uis, allow naming CV clips as well (SynthstromAudible#3183)" This reverts commit 0d55ca5. * Bugfix for rename-UIs PR Co-Authored-By: Nikodemus Siivola <nikodemus@random-state.net> Co-Authored-By: Katherine Whitlock <122173759+stellar-aria@users.noreply.github.com>

refactor rename-uis, allow naming CV clips as well (SynthstromAudible#3183) * refactor rename-uis, allow naming CV clips as well - allow deleting names (by replacing them with empty) for all but drums - consolidate shared code in RenameUI class: subclasses implement only a couple of methods to specialize their behaviour - ad-hoc dispatch in sound_editor.cpp replaced with normal shortcut machinery - shrinks the binary by just a hair, but *shrinks*, not grows! * fix initialization of RenameUIs --------- Co-authored-by: Sapphire Koser <srkoser+GitHub@gmail.com> Fix rename-UIs PR (SynthstromAudible#3218) * Revert "refactor rename-uis, allow naming CV clips as well (SynthstromAudible#3183)" This reverts commit 0d55ca5. * Bugfix for rename-UIs PR follow-up to loosening clip-naming restrictions (SynthstromAudible#3056) - move Song::getClipFromName() to Output, so that we actually check the right thing -- previously if tracks 1 & 2 both have a clip named A, the check might find the clip from track 2, allowing a duplicate name on track 1. Add some things to StringBuf (SynthstromAudible#3204) Co-Authored-By: Nikodemus Siivola <nikodemus@random-state.net> Co-Authored-By: Katherine Whitlock <122173759+stellar-aria@users.noreply.github.com>

refactor rename-uis, allow naming CV clips as well (SynthstromAudible#3183) * refactor rename-uis, allow naming CV clips as well - allow deleting names (by replacing them with empty) for all but drums - consolidate shared code in RenameUI class: subclasses implement only a couple of methods to specialize their behaviour - ad-hoc dispatch in sound_editor.cpp replaced with normal shortcut machinery - shrinks the binary by just a hair, but *shrinks*, not grows! * fix initialization of RenameUIs --------- Co-authored-by: Sapphire Koser <srkoser+GitHub@gmail.com> Fix rename-UIs PR (SynthstromAudible#3218) * Revert "refactor rename-uis, allow naming CV clips as well (SynthstromAudible#3183)" This reverts commit 0d55ca5. * Bugfix for rename-UIs PR follow-up to loosening clip-naming restrictions (SynthstromAudible#3056) - move Song::getClipFromName() to Output, so that we actually check the right thing -- previously if tracks 1 & 2 both have a clip named A, the check might find the clip from track 2, allowing a duplicate name on track 1. Add some things to StringBuf (SynthstromAudible#3204) reset the string to the new value (SynthstromAudible#3315) Co-Authored-By: Nikodemus Siivola <nikodemus@random-state.net> Co-Authored-By: Katherine Whitlock <122173759+stellar-aria@users.noreply.github.com>

nikodemus requested a review from seangoodvibes December 30, 2024 20:15

nikodemus force-pushed the pr-refactor-rename-uis branch from 45a1fbf to ec1e87e Compare December 30, 2024 20:16

nikodemus force-pushed the pr-refactor-rename-uis branch from ec1e87e to 332eba3 Compare December 30, 2024 20:30

sapphire-arches reviewed Jan 1, 2025

View reviewed changes

Comment thread src/deluge/gui/ui/rename/rename_drum_ui.h Outdated

nikodemus and others added 2 commits January 2, 2025 11:27

fix initialization of RenameUIs

863eaf8

Co-authored-by: Sapphire Koser <srkoser+GitHub@gmail.com>

nikodemus force-pushed the pr-refactor-rename-uis branch from 0ff058e to 863eaf8 Compare January 2, 2025 10:00

nikodemus requested a review from sapphire-arches January 2, 2025 10:02

sapphire-arches approved these changes Jan 2, 2025

View reviewed changes

sapphire-arches added this pull request to the merge queue Jan 2, 2025

Merged via the queue into SynthstromAudible:community with commit 0d55ca5 Jan 2, 2025

nikodemus mentioned this pull request Jan 2, 2025

Accessing a parameter shortcut in a Clip then using the NAME shortcut will have shortcut pads blinking while keyboard is visible. #3081

Closed

nikodemus deleted the pr-refactor-rename-uis branch January 3, 2025 09:58

stellar-aria added a commit that referenced this pull request Jan 6, 2025

Revert "refactor rename-uis, allow naming CV clips as well (#3183)"

d350260

This reverts commit 0d55ca5.

github-merge-queue Bot pushed a commit that referenced this pull request Jan 6, 2025

Fix rename-UIs PR (#3218)

baf20ed

* Revert "refactor rename-uis, allow naming CV clips as well (#3183)" This reverts commit 0d55ca5. * Bugfix for rename-UIs PR

Navigation Menu

Uh oh!

Uh oh!

refactor rename-uis, allow naming CV clips as well - #3183

refactor rename-uis, allow naming CV clips as well#3183
sapphire-arches merged 2 commits into
SynthstromAudible:communityfrom
nikodemus:pr-refactor-rename-uis

nikodemus commented Dec 30, 2024

Uh oh!

github-actions Bot commented Dec 30, 2024 •

edited

Loading

Uh oh!

Uh oh!

sapphire-arches commented Jan 2, 2025

Uh oh!

nikodemus commented Jan 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

nikodemus commented Dec 30, 2024

Uh oh!

github-actions Bot commented Dec 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Uh oh!

sapphire-arches commented Jan 2, 2025

Uh oh!

nikodemus commented Jan 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Dec 30, 2024 •

edited

Loading