-
Notifications
You must be signed in to change notification settings - Fork 46
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
Add project and take markers action #1133
base: master
Are you sure you want to change the base?
Changes from 4 commits
7b59a65
6eb4409
b0c54f8
03a1679
cb90dfb
19a9474
5a531e5
8c0662e
ca63f2c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -4793,8 +4793,33 @@ void cmdInsertMarker(Command* command) { | |||||
Main_OnCommand(command->gaccel.accel.cmd, 0); | ||||||
return; | ||||||
} | ||||||
if(CountSelectedMediaItems(0)>0) { | ||||||
vector<int> preMarkers(CountSelectedMediaItems(nullptr)); | ||||||
ScottChesworth marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
for(int i = 0; i < preMarkers.size(); ++ i) { | ||||||
MediaItem* item = GetSelectedMediaItem(nullptr, i); | ||||||
MediaItem_Take* take = GetActiveTake(item); | ||||||
preMarkers[i] = GetNumTakeMarkers(take); | ||||||
} | ||||||
Main_OnCommand(42390, 0); // Item: Quick add take marker at play position or edit cursor | ||||||
int addedMarkers = 0; | ||||||
for(int i = 0; i < preMarkers.size(); ++ i) { | ||||||
MediaItem* item = GetSelectedMediaItem(nullptr, i); | ||||||
MediaItem_Take* take = GetActiveTake(item); | ||||||
if (preMarkers[i] < GetNumTakeMarkers(take)) { | ||||||
++ addedMarkers; | ||||||
} | ||||||
} | ||||||
if (addedMarkers == 0) { | ||||||
return; // not inserted | ||||||
} else { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no need for an else block if you always return in the if above it. This also simplifies things by avoiding a level of nesting. |
||||||
// Translators: Reported when using REAPER's quick add take marker action. If more than one take marker is inserted, [] will be replaced with the number of markers. E.G. "2 take markers inserted". | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
outputMessage(format( | ||||||
translate_plural("take marker inserted", "{} take markers inserted", addedMarkers), addedMarkers)); | ||||||
return; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no need for a return if it's the last thing in the function and the function returns nothing.
Suggested change
|
||||||
} | ||||||
} | ||||||
int count = CountProjectMarkers(nullptr, nullptr, nullptr); | ||||||
Main_OnCommand(command->gaccel.accel.cmd, 0); | ||||||
Main_OnCommand(40157, 0); | ||||||
if (CountProjectMarkers(nullptr, nullptr, nullptr) == count) { | ||||||
return; // Not inserted. | ||||||
} | ||||||
|
@@ -4810,6 +4835,20 @@ void cmdInsertMarker(Command* command) { | |||||
outputMessage(format(translate("marker {} inserted"), number)); | ||||||
} | ||||||
|
||||||
void cmdInsertOrEditMarker(Command* command) { | ||||||
double start, end; | ||||||
GetSet_LoopTimeRange(false, true, &start, &end, false); | ||||||
if (start != end && CountSelectedMediaItems(nullptr)>0) { | ||||||
Main_OnCommand(43181, 0); // Item: Add/edit take marker at time selection | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The action is called "OSARA: Add project or take marker at cursor (depending on focus)". It doesn't mention time selection. Should it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Urgh, just realised that I renamed the wrong action. Time selection is only relevant when using the add/edit one. Another commit coming up shortly. |
||||||
return; | ||||||
} else if(CountSelectedMediaItems(nullptr) > 0) { | ||||||
Main_OnCommand(42385, 0); // Item: Add/edit take marker at play position or edit cursor | ||||||
} else { | ||||||
Main_OnCommand(40171, 0); // Markers: Insert and/or edit marker at current position | ||||||
return; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unnecessary return.
Suggested change
|
||||||
} | ||||||
} | ||||||
|
||||||
void cmdInsertRegion(Command* command) { | ||||||
if (!shouldReportTimeMovement()) { | ||||||
Main_OnCommand(command->gaccel.accel.cmd, 0); | ||||||
|
@@ -5252,6 +5291,8 @@ Command COMMANDS[] = { | |||||
{MAIN_SECTION, {DEFACCEL, _t("OSARA: Check for update")}, "OSARA_UPDATE", cmdCheckForUpdate}, | ||||||
{MAIN_SECTION, {DEFACCEL, _t("OSARA: Open online documentation")}, "OSARA_OPENDOC", cmdOpenDoc}, | ||||||
{MAIN_SECTION, {DEFACCEL, _t("OSARA: Report tempo and time signature at play cursor; press twice to add/edit tempo markers")}, "OSARA_MANAGETEMPOTIMESIGMARKERS", cmdManageTempoTimeSigMarkers}, | ||||||
{MAIN_SECTION, {DEFACCEL, _t("OSARA: Add project or take marker at cursor (depending on focus)")}, "OSARA_ADDPROJTAKEMARKER", cmdInsertMarker}, | ||||||
{MAIN_SECTION, {DEFACCEL, _t("OSARA: Add/edit project or take marker at cursor (depending on focus)")}, "OSARA_ADDEDITPROJTAKEMARKER", cmdInsertOrEditMarker}, | ||||||
{MIDI_EDITOR_SECTION, {DEFACCEL, _t("OSARA: Enable noncontiguous selection/toggle selection of current chord/note")}, "OSARA_MIDITOGGLESEL", cmdMidiToggleSelection}, | ||||||
{MIDI_EDITOR_SECTION, {DEFACCEL, _t("OSARA: Move to next chord")}, "OSARA_NEXTCHORD", cmdMidiMoveToNextChord}, | ||||||
{MIDI_EDITOR_SECTION, {DEFACCEL, _t("OSARA: Move to previous chord")}, "OSARA_PREVCHORD", cmdMidiMoveToPreviousChord}, | ||||||
|
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.
If there are no selected items, this means we will never pass the command to REAPER. Practically, this should be fine, since REAPER will probably do nothing in the case of 0 selected items. However, when intercepting commands, I do think we should always pass them to REAPER, on the very slight chance that REAPER does something we don't expect, either now or in future. Otherwise, the action will just do nothing, even if it would have done something without OSARA installed.
You could either: