Address #513: Use hotkeys everywhere

The-Orm/KeyboardMacroView.cpp

@@ -17,6 +17,9 @@
 #include <spdlog/spdlog.h>
 #include "SpdLogJuce.h"
 
+#include <atomic>
+#include <utility>
+
 // Standardize text
 const char *kMacrosEnabled = "Macros enabled";
 const char *kAutomaticSetup = "Use current synth as master";
@@ -31,8 +34,34 @@ const char *kMidiChannel = "MIDI channel";
 const char *kLowestNote = "Lowest MIDI Note";
 const char *kHighestNote = "Highest MIDI Note";
 
+namespace {
+class KeyMappingDialogContent : public juce::Component {
+public:
+	explicit KeyMappingDialogContent(juce::ApplicationCommandManager& commandManager)
+	{
+		auto* mappings = commandManager.getKeyMappings();
+		jassert(mappings != nullptr);
+		if (mappings != nullptr) {
+			keyEditor_ = std::make_unique<juce::KeyMappingEditorComponent>(*mappings, true);
+			addAndMakeVisible(*keyEditor_);
+		}
+		setSize(900, 520);
+	}
+
+	void resized() override
+	{
+		if (keyEditor_) {
+			keyEditor_->setBounds(getLocalBounds().reduced(8));
+		}
+	}
+
+private:
+	std::unique_ptr<juce::KeyMappingEditorComponent> keyEditor_;
+};
+}
+
 
-class KeyboardMacroView::RecordProgress : private MidiKeyboardStateListener {
+class KeyboardMacroView::RecordProgress : private MidiKeyboardStateListener, public std::enable_shared_from_this<KeyboardMacroView::RecordProgress> {
 public:
 	RecordProgress(Component* parent, MidiKeyboardState& state) : parent_(parent), state_(state), atLeastOneKey_(false)
 	{
@@ -42,15 +71,20 @@ class KeyboardMacroView::RecordProgress : private MidiKeyboardStateListener {
 		done_ = std::move(done);
 		auto options = juce::MessageBoxOptions().withButton("Clear").withButton("Cancel").withTitle("Press key(s) on your MIDI keyboard").withParentComponent(parent_);
 		state_.addListener(this);
-		messageBox_ = AlertWindow::showScopedAsync(options, [this](int button) {
+		auto weakSelf = weak_from_this();
+		messageBox_ = AlertWindow::showScopedAsync(options, [weakSelf](int button) {
+			auto self = weakSelf.lock();
+			if (!self) {
+				return;
+			}
 			switch (button) {
 			case 1:
 				// Clear
-				done_({}, false);
+				self->finish({}, false);
 				break;
 			case 0:
 				// Cancel, nothing to do
-				done_({}, true);
+				self->finish({}, true);
 				break;
 			default:
 				spdlog::error("Unknown button number pressed, program error in RecordProgress of KeyboardMacroView");
@@ -64,6 +98,9 @@ class KeyboardMacroView::RecordProgress : private MidiKeyboardStateListener {
 
 	virtual void handleNoteOn(MidiKeyboardState* source, int midiChannel, int midiNoteNumber, float velocity) override {
 		ignoreUnused(source, midiChannel, velocity);
+		if (completionQueued_) {
+			return;
+		}
 		notes_.insert(midiNoteNumber);
 		atLeastOneKey_ = true;
 	}
@@ -76,48 +113,79 @@ class KeyboardMacroView::RecordProgress : private MidiKeyboardStateListener {
 				keyPressed++;
 			}
 		}
-		if (keyPressed == 0) {
-			messageBox_.close();
-			done_(notes_, false);
+		if (atLeastOneKey_ && keyPressed == 0 && !completionQueued_.exchange(true)) {
+			// MIDI callbacks must not close dialogs or update settings. Keep a copy
+			// of the recorded chord and discard delivery if the recorder is gone.
+			MessageManager::callAsync([weakSelf = weak_from_this(), notes = notes_]() {
+				if (auto self = weakSelf.lock()) {
+					self->finish(notes, false);
+				}
+			});
 		}
 	}
 
 private:
+	void finish(std::set<int> const& notes, bool cancelled) {
+		jassert(MessageManager::getInstance()->isThisTheMessageThread());
+		completionQueued_ = true;
+		// Clear/Cancel and an already queued note-off may both arrive. Consume
+		// the callback before closing the dialog so completion is exactly once.
+		auto done = std::exchange(done_, {});
+		if (done) {
+			messageBox_.close();
+			done(notes, cancelled);
+		}
+	}
+
 	Component* parent_;
 	std::function<void(std::set<int> const&, bool)> done_;
 	ScopedMessageBox messageBox_;
 	std::set<int> notes_;
 	MidiKeyboardState &state_;
 	bool atLeastOneKey_;
+	std::atomic<bool> completionQueued_{ false };
 
 };
 
-KeyboardMacroView::KeyboardMacroView(std::function<void(KeyboardMacroEvent)> callback) : keyboard_(state_, MidiKeyboardComponent::horizontalKeyboard), executeMacro_(callback)
+KeyboardMacroView::KeyboardMacroView(std::function<void(KeyboardMacroEvent)> executeCallback,
+	juce::ApplicationCommandManager& commandManager)
+	: keyboard_(state_, MidiKeyboardComponent::horizontalKeyboard),
+	  keyboardShortcutsButton_("Keyboard Shortcuts..."),
+	  executeMacro_(std::move(executeCallback)),
+	  commandManager_(commandManager)
 {
 	addAndMakeVisible(customSetup_);
 	addAndMakeVisible(keyboard_);
 	keyboard_.setOctaveForMiddleC(4); // This is correct for the DSI Synths, I just don't know what the standard is
+	addAndMakeVisible(keyboardShortcutsButton_);
+	keyboardShortcutsButton_.onClick = [this]() {
+		showKeyboardShortcutEditor();
+	};
 	addAndMakeVisible(macroViewport_);
 	macroContainer_ = std::make_unique<Component>();
 	macroViewport_.setScrollBarsShown(true, false);
 	macroViewport_.setViewedComponent(macroContainer_.get(), false);
 
 	// Create config table
 	for (auto config : kAllKeyboardMacroEvents) {
-		auto configComponent = new MacroConfig(config,
-			[this](KeyboardMacroEvent event) {
-				activeRecorder_ = std::make_shared<RecordProgress>(this, state_);
-				activeRecorder_->show([this, event](std::set<int> const& notes, bool cancelled) {
-					if (!cancelled) {
-						KeyboardMacro newMacro = { event, notes };
-						macros_[event] = newMacro;
-						saveSettings();
-						refreshUI();
-					}
-					activeRecorder_ = nullptr;
-					}
-				);
-		},
+			auto configComponent = new MacroConfig(config,
+				[this](KeyboardMacroEvent event) {
+					juce::Component::SafePointer<KeyboardMacroView> safeThis(this);
+					activeRecorder_ = std::make_shared<RecordProgress>(this, state_);
+					activeRecorder_->show([safeThis, event](std::set<int> const& notes, bool cancelled) {
+						if (!safeThis) {
+							return;
+						}
+						if (!cancelled) {
+							KeyboardMacro newMacro = { event, notes };
+							safeThis->macros_[event] = newMacro;
+							safeThis->saveSettings();
+						}
+						safeThis->activeRecorder_ = nullptr;
+						safeThis->refreshUI();
+						}
+					);
+				},
 			[this](KeyboardMacroEvent event, bool down) {
 			if (macros_.find(event) != macros_.end()) {
 				for (auto key : macros_[event].midiNotes) {
@@ -183,15 +251,19 @@ KeyboardMacroView::KeyboardMacroView(std::function<void(KeyboardMacroEvent)> cal
 				for (const auto& macro : macros_) {
 					bool matched = isMacroState(macro.second);
 					bool wasActive = macroActiveStates_[macro.first];
-					if (matched && !wasActive) {
-						macroActiveStates_[macro.first] = true;
-						auto code = macro.first;
-						MessageManager::callAsync([this, code]() {
-							executeMacro_(code);
-							});
-					} else if (!matched && wasActive) {
-						macroActiveStates_[macro.first] = false;
-					}
+						if (matched && !wasActive) {
+							macroActiveStates_[macro.first] = true;
+							auto code = macro.first;
+							juce::Component::SafePointer<KeyboardMacroView> safeThis(this);
+							MessageManager::callAsync([safeThis, code]() {
+								if (!safeThis) {
+									return;
+								}
+								safeThis->executeMacro_(code);
+								});
+						} else if (!matched && wasActive) {
+							macroActiveStates_[macro.first] = false;
+						}
 				}
 			}
 			else if (message.isControllerOfType(123)) {
@@ -213,6 +285,11 @@ KeyboardMacroView::KeyboardMacroView(std::function<void(KeyboardMacroEvent)> cal
 
 KeyboardMacroView::~KeyboardMacroView()
 {
+	if (keyMappingDialog_ != nullptr) {
+		keyMappingDialog_->exitModalState(0);
+		keyMappingDialog_ = nullptr;
+	}
+	activeRecorder_ = nullptr;
 	midikraft::MidiController::instance()->removeMessageHandler(handle_);
 	saveSettings();
 }
@@ -259,13 +336,33 @@ void KeyboardMacroView::refreshUI() {
 	// Set UI
 	int i = 0;
 	for (auto config : kAllKeyboardMacroEvents) {
+		KeyboardMacro macroData{ config, {} };
 		if (macros_.find(config) != macros_.end()) {
-			configs_[i]->setData(macros_[config]);
+			macroData = macros_[config];
 		}
+		configs_[i]->setData(macroData);
 		i++;
 	}
 }
 
+void KeyboardMacroView::showKeyboardShortcutEditor()
+{
+	if (keyMappingDialog_ != nullptr) {
+		keyMappingDialog_->toFront(true);
+		return;
+	}
+
+	juce::DialogWindow::LaunchOptions options;
+	options.dialogTitle = "Keyboard Shortcuts";
+	options.content.setOwned(new KeyMappingDialogContent(commandManager_));
+	options.componentToCentreAround = this;
+	options.escapeKeyTriggersCloseButton = true;
+	options.useNativeTitleBar = true;
+	options.resizable = true;
+	options.useBottomRightCornerResizer = true;
+	keyMappingDialog_ = options.launchAsync();
+}
+
 void setMidiDeviceFromString(const std::shared_ptr<TypedNamedValue>& prop, const std::string& storedValue, bool allowAppend = false) {
 	if (prop) {
 		auto midiDeviceProp = std::dynamic_pointer_cast<MidiDevicePropertyEditor>(prop);
@@ -289,6 +386,7 @@ void setMidiDeviceFromString(const std::shared_ptr<TypedNamedValue>& prop, const
 
 
 void KeyboardMacroView::loadFromSettings() {
+	macros_.clear();
 	auto json = Settings::instance().get("MacroDefinitions");
 	if (!json.empty()) {
 		try {
@@ -311,8 +409,10 @@ void KeyboardMacroView::loadFromSettings() {
 							std::string eventString = event;
 							macroEventCode = KeyboardMacro::fromText(eventString);
 						}
-						if (macroEventCode != KeyboardMacroEvent::Unknown && !midiNoteValues.empty()) {
-							macros_[macroEventCode] = { macroEventCode, midiNoteValues };
+						if (macroEventCode != KeyboardMacroEvent::Unknown) {
+							if (!midiNoteValues.empty()) {
+								macros_[macroEventCode] = { macroEventCode, midiNoteValues };
+							}
 						}
 					}
 				}
@@ -358,14 +458,21 @@ void KeyboardMacroView::loadFromSettings() {
 void KeyboardMacroView::saveSettings() {
 	var result;
 
-	for (auto macro : macros_) {
+	for (auto event : kAllKeyboardMacroEvents) {
+		bool hasMidi = macros_.find(event) != macros_.end() && !macros_[event].midiNotes.empty();
+		if (!hasMidi) {
+			continue;
+		}
+
 		var notes;
-		for (auto note : macro.second.midiNotes) {
-			notes.append(note);
+		if (hasMidi) {
+			for (auto note : macros_[event].midiNotes) {
+				notes.append(note);
+			}
 		}
 		auto def = new DynamicObject();
 		def->setProperty("Notes", notes);
-		def->setProperty("Event", String(KeyboardMacro::toText(macro.first)));
+		def->setProperty("Event", String(KeyboardMacro::toText(event)));
 		result.append(def);
 	}
 	String json = JSON::toString(result);
@@ -415,13 +522,17 @@ void KeyboardMacroView::resized()
 
 	customSetup_.setBounds(leftColumn);
 
+	auto shortcutButtonArea = rightColumn.removeFromTop(LAYOUT_LINE_SPACING);
+	keyboardShortcutsButton_.setBounds(shortcutButtonArea.removeFromRight(220));
+	rightColumn.removeFromTop(LAYOUT_INSET_NORMAL / 2);
+
 	// Config table in scroll area on the right
 	macroViewport_.setBounds(rightColumn);
 	const int scrollWidth = macroViewport_.getLocalBounds().getWidth();
 	const int rowWidth = std::max(0, scrollWidth - 2 * LAYOUT_INSET_NORMAL);
 	const int rowX = (scrollWidth - rowWidth) / 2;
 	int y = 0;
-	const int rowHeight = LAYOUT_LINE_SPACING; // Match property editor vertical rhythm
+	const int rowHeight = LAYOUT_LINE_SPACING;
 	for (auto c : configs_) {
 		auto row = Rectangle<int>(rowX, y, rowWidth, rowHeight);
 		c->setBounds(row);

The-Orm/KeyboardMacroView.h

@@ -19,7 +19,8 @@
 
 class KeyboardMacroView : public Component, private ChangeListener, private Value::Listener {
 public:
-	KeyboardMacroView(std::function<void(KeyboardMacroEvent)> callback);
+	KeyboardMacroView(std::function<void(KeyboardMacroEvent)> executeCallback,
+		juce::ApplicationCommandManager& commandManager);
 	virtual ~KeyboardMacroView() override;
 
 	virtual void resized() override;
@@ -37,6 +38,7 @@ class KeyboardMacroView : public Component, private ChangeListener, private Valu
 	bool isMacroState(KeyboardMacro const &macro);
 	void refreshSynthList();
 	void refreshUI();
+	void showKeyboardShortcutEditor();
 
 	void turnOnMasterkeyboardInput();
 
@@ -47,6 +49,7 @@ class KeyboardMacroView : public Component, private ChangeListener, private Valu
 	MidiKeyboardState state_;
 	MidiKeyboardComponent keyboard_;
 	Viewport macroViewport_;
+	TextButton keyboardShortcutsButton_;
 	std::unique_ptr<Component> macroContainer_;
 	std::shared_ptr<MidiDevicePropertyEditor> midiDeviceList_; // Listen to this to get notified of newly available devices!
 	std::shared_ptr<MidiDevicePropertyEditor> secondaryMidiOutList_;
@@ -57,6 +60,8 @@ class KeyboardMacroView : public Component, private ChangeListener, private Valu
 
 	std::map<KeyboardMacroEvent, KeyboardMacro> macros_;
 	std::function<void(KeyboardMacroEvent)> executeMacro_;
+	juce::ApplicationCommandManager& commandManager_;
+	juce::Component::SafePointer<juce::DialogWindow> keyMappingDialog_;
 	std::map<KeyboardMacroEvent, bool> macroActiveStates_; // Tracks edge-trigger state to avoid repeats while held
 
 	midikraft::MidiController::HandlerHandle handle_ = midikraft::MidiController::makeNoneHandle();

The-Orm/MacroConfig.cpp

@@ -58,9 +58,9 @@ KeyboardMacroEvent KeyboardMacro::fromText(std::string const &event)
 	return KeyboardMacroEvent::Unknown;
 }
 
-MacroConfig::MacroConfig(KeyboardMacroEvent event, 
+MacroConfig::MacroConfig(KeyboardMacroEvent event,
 	std::function<void(KeyboardMacroEvent)> recordHander,
-	std::function<void(KeyboardMacroEvent, bool)> showHandler) : event_(event), 
+	std::function<void(KeyboardMacroEvent, bool)> showHandler) : event_(event),
 	recordHander_(recordHander),
 	showHandler_(showHandler), play_([this](TextButton *button) { buttonStateChanged(button);  }) // NOLINT
 {

The-Orm/MacroConfig.h

@@ -36,7 +36,9 @@ class MacroConfig : public Component,
 	private TextButton::Listener
 {
 public:
-	MacroConfig(KeyboardMacroEvent event, std::function<void(KeyboardMacroEvent)> recordHander, std::function<void(KeyboardMacroEvent, bool)> showHandler);
+	MacroConfig(KeyboardMacroEvent event,
+		std::function<void(KeyboardMacroEvent)> recordHander,
+		std::function<void(KeyboardMacroEvent, bool)> showHandler);
 
 	virtual void resized() override;
 
@@ -55,4 +57,3 @@ class MacroConfig : public Component,
 	MouseUpAndDownButton play_;
 };
 
-