Respond to review

Author: CMurtagh-LGTM

src/wayland/input_method/input_method.cpp

@@ -4,7 +4,6 @@
 #include <qdebug.h>
 #include <qlogging.h>
 #include <qobject.h>
-#include <qpointer.h>
 #include <qstring.h>
 #include <qtmetamacros.h>
 #include <qwayland-input-method-unstable-v2.h>
@@ -21,12 +20,12 @@ InputMethodHandle::InputMethodHandle(QObject* parent, ::zwp_input_method_v2* inp
     , zwp_input_method_v2(inputMethod) {}
 
 InputMethodHandle::~InputMethodHandle() {
-	this->sendPreeditString("");
+	this->setPreeditString("");
 	this->destroy();
 }
 
 void InputMethodHandle::commitString(const QString& text) { this->commit_string(text); }
-void InputMethodHandle::sendPreeditString(
+void InputMethodHandle::setPreeditString(
     const QString& text,
     int32_t cursorBegin,
     int32_t cursorEnd
@@ -39,7 +38,7 @@ void InputMethodHandle::deleteText(int before, int after) {
 void InputMethodHandle::commit() { this->zwp_input_method_v2::commit(this->serial++); }
 
 bool InputMethodHandle::hasKeyboard() const { return this->keyboard != nullptr; }
-QPointer<InputMethodKeyboardGrab> InputMethodHandle::grabKeyboard() {
+InputMethodKeyboardGrab* InputMethodHandle::grabKeyboard() {
 	if (this->keyboard) return this->keyboard;
 
 	this->keyboard = new InputMethodKeyboardGrab(this, this->grab_keyboard());
@@ -48,7 +47,7 @@ QPointer<InputMethodKeyboardGrab> InputMethodHandle::grabKeyboard() {
 }
 void InputMethodHandle::releaseKeyboard() {
 	if (!this->keyboard) return;
-	this->keyboard->deleteLater();
+	delete this->keyboard;
 	this->keyboard = nullptr;
 }
 

src/wayland/input_method/input_method.hpp

@@ -2,7 +2,6 @@
 
 #include <cstdint>
 #include <qobject.h>
-#include <qpointer.h>
 #include <qtclasshelpermacros.h>
 #include <qwayland-input-method-unstable-v2.h>
 
@@ -25,12 +24,12 @@ class InputMethodHandle
 
 	void commitString(const QString& text);
 	// By default hides the cursor
-	void sendPreeditString(const QString& text, int32_t cursorBegin = -1, int32_t cursorEnd = -1);
+	void setPreeditString(const QString& text, int32_t cursorBegin = -1, int32_t cursorEnd = -1);
 	void deleteText(int before, int after);
 	void commit();
 
 	[[nodiscard]] bool hasKeyboard() const;
-	QPointer<InputMethodKeyboardGrab> grabKeyboard();
+	InputMethodKeyboardGrab* grabKeyboard();
 	void releaseKeyboard();
 
 	[[nodiscard]] bool isActive() const;

src/wayland/input_method/keyboard_grab.cpp

@@ -16,10 +16,13 @@
 #include <xkbcommon/xkbcommon-keysyms.h>
 #include <xkbcommon/xkbcommon.h>
 
+#include "../../core/logcat.hpp"
 #include "manager.hpp"
 #include "types.hpp"
 #include "virtual_keyboard.hpp"
 
+QS_LOGGING_CATEGORY(inputMethodKeyboardKeys, "quickshell.wayland.inputMethod.keyboardKeys", QtWarningMsg);
+
 namespace qs::wayland::input_method::impl {
 
 InputMethodKeyboardGrab::InputMethodKeyboardGrab(
@@ -91,11 +94,9 @@ void InputMethodKeyboardGrab::zwp_input_method_keyboard_grab_v2_key(
 
 	key += WAYLAND_KEY_OFFSET;
 
-#if INPUT_METHOD_PRINT
-	qDebug() << KeyMapState::keyStateName(static_cast<wl_keyboard_key_state>(state))
+	qCInfo(inputMethodKeyboardKeys) << KeyMapState::keyStateName(static_cast<wl_keyboard_key_state>(state))
 	         << this->mKeyMapState.keyName(key) << "[" << key << "]"
 	         << this->mKeyMapState.getChar(key);
-#endif
 
 	const xkb_keysym_t sym = this->mKeyMapState.getOneSym(key);
 

src/wayland/input_method/manager.cpp

@@ -15,8 +15,12 @@ InputMethodManager::InputMethodManager(): QWaylandClientExtensionTemplate(1) { t
 InputMethodManager::~InputMethodManager() { this->destroy(); }
 
 InputMethodManager* InputMethodManager::instance() {
-	// The OS should free this memory when we exit
-	static auto* instance = new InputMethodManager();
+	static InputMethodManager* instance = nullptr;
+
+	if (instance == nullptr) {
+		instance = new InputMethodManager();
+	}
+
 	return instance;
 }
 
@@ -37,7 +41,7 @@ QPointer<InputMethodHandle> InputMethodManager::acquireInput() {
 }
 
 void InputMethodManager::releaseInput() {
-	this->inputMethod->deleteLater();
+	delete this->inputMethod;
 	this->inputMethod = nullptr;
 }
 
@@ -46,8 +50,11 @@ VirtualKeyboardManager::VirtualKeyboardManager(): QWaylandClientExtensionTemplat
 }
 
 VirtualKeyboardManager* VirtualKeyboardManager::instance() {
-	// The OS should free this memory when we exit
-	static auto* instance = new VirtualKeyboardManager();
+	static VirtualKeyboardManager* instance = nullptr;
+
+	if (instance == nullptr) {
+		instance = new VirtualKeyboardManager();
+	}
 	return instance;
 }
 

src/wayland/input_method/qml.cpp

@@ -22,42 +22,37 @@ using namespace impl;
 
 InputMethod::InputMethod(QObject* parent): QObject(parent) { this->getInput(); }
 
-void InputMethod::sendString(const QString& text) {
+void InputMethod::commitString(const QString& text) {
 	if (!this->isActive()) return;
 
 	this->handle->commitString(text);
-	this->handle->commit();
 }
 
-void InputMethod::sendPreeditString(const QString& text, int32_t cursorBegin, int32_t cursorEnd) {
+void InputMethod::setPreeditString() {
 	if (!this->isActive()) return;
 
-	this->handle->sendPreeditString(text, cursorBegin, cursorEnd);
-	this->handle->commit();
+	if (mCursorEnd == -1) {
+		this->handle->setPreeditString(mPreeditString, mCursorBegin, mCursorBegin);
+	} else {
+		this->handle->setPreeditString(mPreeditString, mCursorBegin, mCursorEnd);
+	}
 }
 
-void InputMethod::deleteText(int before, int after) {
+void InputMethod::deleteSuroundingText(int before, int after) {
 	if (!this->isActive()) return;
 
 	this->handle->deleteText(before, after);
-	this->handle->commit();
 }
 
-bool InputMethod::isActive() const { return this->hasInput() && this->handle->isActive(); }
+void InputMethod::commit() {
+	if (!this->isActive()) return;
 
-QQmlComponent* InputMethod::keyboardComponent() const { return this->mKeyboardComponent; }
-void InputMethod::setKeyboardComponent(QQmlComponent* keyboardComponent) {
-	if (this->mKeyboardComponent == keyboardComponent) return;
-	this->mKeyboardComponent = keyboardComponent;
-	emit this->keyboardComponentChanged();
+	this->handle->commit();
+}
 
-	if (this->keyboard) {
-		this->keyboard->deleteLater();
-		this->keyboard = nullptr;
-	}
+bool InputMethod::isActive() const { return this->hasInput() && this->handle->isActive(); }
 
-	this->handleKeyboardActive();
-}
+QPointer<Keyboard> InputMethod::keyboard() const { return this->mKeyboard; }
 
 bool InputMethod::hasInput() const { return this->handle && this->handle->isAvailable(); }
 
@@ -108,10 +103,10 @@ void InputMethod::releaseInput() {
 
 bool InputMethod::hasKeyboard() const {
 	// The lifetime of keyboard should be less than handle's
-	if (this->keyboard) {
+	if (this->mKeyboard) {
 		assert(this->handle->hasKeyboard());
 	}
-	return this->keyboard;
+	return this->mKeyboard;
 }
 
 void InputMethod::grabKeyboard() {
@@ -120,44 +115,71 @@ void InputMethod::grabKeyboard() {
 		qmlDebug(this) << "Only one input method can grad a keyboard at any one time";
 		return;
 	}
-	auto* instanceObj =
-	    this->mKeyboardComponent->create(QQmlEngine::contextForObject(this->mKeyboardComponent));
-	auto* instance = qobject_cast<Keyboard*>(instanceObj);
-
-	if (instance == nullptr) {
-		qWarning() << "Failed to create input method keyboard component";
-		if (instanceObj != nullptr) instanceObj->deleteLater();
-		return;
-	}
+	this->mKeyboard = new Keyboard(this);
 
-	instance->setParent(this);
-	instance->setKeyboard(this->handle->grabKeyboard());
+	this->mKeyboard->setKeyboard(this->handle->grabKeyboard());
 	// Always have a way to release the keyboard
-	QObject::connect(instance, &Keyboard::escapePress, this, &InputMethod::releaseKeyboard);
+	QObject::connect(this->mKeyboard, &Keyboard::escapePress, this, &InputMethod::releaseKeyboard);
 
-	this->keyboard = instance;
 	emit this->hasKeyboardChanged();
 }
 
 void InputMethod::releaseKeyboard() {
 	if (!this->hasKeyboard()) return;
-	this->keyboard->deleteLater();
-	this->keyboard = nullptr;
+	delete this->mKeyboard;
+	this->mKeyboard = nullptr;
 	this->handle->releaseKeyboard();
-	if (this->mClearPreeditOnKeyboardRelease) this->sendPreeditString("");
+	if (this->mClearPreeditOnKeyboardRelease) {
+		this->mPreeditString = "";
+		this->mCursorBegin = -1;
+		this->mCursorEnd = -1;
+		this->setPreeditString();
+		this->preeditStringChanged();
+		this->cursorBeginChanged();
+		this->cursorEndChanged();
+	}
 	emit this->hasKeyboardChanged();
 }
 
+const QString& InputMethod::preeditString() const { return this->mPreeditString; }
+QString& InputMethod::preeditString() { return this->mPreeditString; }
+void InputMethod::setPreeditString(const QString& string) {
+	if (this->mPreeditString == string) return;
+	this->mPreeditString = string;
+	this->setPreeditString();
+	this->commit();
+	this->preeditStringChanged();
+}
+int32_t InputMethod::cursorBegin() const { return this->mCursorBegin; }
+void InputMethod::setCursorBegin(int32_t position) {
+	if (this->mCursorBegin == position) return;
+	this->mCursorBegin = position;
+	this->setPreeditString();
+	this->commit();
+	this->cursorBeginChanged();
+}
+int32_t InputMethod::cursorEnd() const { return this->mCursorEnd; }
+void InputMethod::setCursorEnd(int32_t position) {
+	if (this->mCursorEnd == position) return;
+	this->mCursorEnd = position;
+	this->setPreeditString();
+	this->commit();
+	this->cursorEndChanged();
+}
+
 void InputMethod::handleKeyboardActive() {
-	if (!this->mKeyboardComponent) return;
-	if (this->keyboard) {
+	if (this->mKeyboard) {
 		this->releaseKeyboard();
 	}
 }
 
 const QString& InputMethod::surroundingText() const { return this->handle->surroundingText(); }
-uint32_t InputMethod::surroundingTextCursor() const { return this->handle->surroundingTextCursor(); }
-uint32_t InputMethod::surroundingTextAnchor() const { return this->handle->surroundingTextAnchor(); }
+uint32_t InputMethod::surroundingTextCursor() const {
+	return this->handle->surroundingTextCursor();
+}
+uint32_t InputMethod::surroundingTextAnchor() const {
+	return this->handle->surroundingTextAnchor();
+}
 
 QMLContentHint::Enum InputMethod::contentHint() const { return this->handle->contentHint(); }
 QMLContentPurpose::Enum InputMethod::contentPurpose() const {