From kde-core-devel  Mon Aug 20 20:43:42 2007
From: Thomas McGuire <thomas.mcguire () gmx ! net>
Date: Mon, 20 Aug 2007 20:43:42 +0000
To: kde-core-devel
Subject: [PATCH] New signal for KKeySequenceWidget
Message-Id: <200708202243.42892.thomas.mcguire () gmx ! net>
X-MARC-Message: https://marc.info/?l=kde-core-devel&m=118764725217240
MIME-Version: 1
Content-Type: multipart/mixed; boundary="--Boundary-00=_+zfyGfXnFsYxAcM"

--Boundary-00=_+zfyGfXnFsYxAcM
Content-Type: text/plain;
  charset="us-ascii"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

Hello,

attached a patch which adds a new signal to KKeySequenceWidget. This signal is 
like the existing keySequenceChanged() signal, just with more parameters.

The new parameters include the old key sequence and a boolean flag wether the 
key sequence was changed programmatically or by the user.

This is necessary to correctly port the usages of KKeySequenceWidget. 
Currently, the code using the widget is incorrect, because it relies on the 
fact that the new key sequence is not yet set when the signal is emitted. 
This is however not the case anymore with the new KKeySequenceWidget.
Because of this, reverting to the old shortcut if a conflict occurs is not 
possible.

A quick look at lxr.kde.org reveals that all users outside of KDEUI are 
currently incorrect and could benefit from the new signal.

Attached also a sample change of one of the four usages in KMail.

OK to commit next Monday (27th)?
(if there are no objections, I'll commit)

Regards,
Thomas

--Boundary-00=_+zfyGfXnFsYxAcM
Content-Type: text/x-diff;
  charset="us-ascii";
  name="kksw.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename="kksw.diff"

Index: kkeysequencewidget.cpp
===================================================================
--- kkeysequencewidget.cpp	(revision 702289)
+++ kkeysequencewidget.cpp	(working copy)
@@ -90,6 +90,7 @@
 
 //private slot
 	void doneRecording();
+	void updateSequence(bool programmatically);
 	
 //members
 	KKeySequenceWidget *const q;
@@ -187,7 +188,7 @@
 {
 	d->oldKeySequence = d->keySequence;
 	d->keySequence = seq;
-	d->doneRecording();
+	d->updateSequence(true);
 }
 
 
@@ -205,6 +206,25 @@
 }
 
 
+void KKeySequenceWidgetPrivate::updateSequence(bool programmatically)
+{
+	modifierlessTimeout.stop();
+	isRecording = false;
+	keyButton->releaseKeyboard();
+	keyButton->setDown(false);
+	updateShortcutDisplay();
+	if (keySequence != oldKeySequence) {
+		//Note that we need to create a copy of the QKeySequence here,
+		//because the receiving slot might call setKeySequence() with
+		//these key sequences.
+		emit q->keySequenceChanged(QKeySequence(keySequence));
+		emit q->keySequenceChanged(QKeySequence(keySequence),
+                                           QKeySequence(oldKeySequence),
+					   programmatically);
+	}
+}
+
+
 void KKeySequenceWidgetPrivate::startRecording()
 {
 	nKey = 0;
@@ -220,13 +240,7 @@
 
 void KKeySequenceWidgetPrivate::doneRecording()
 {
-	modifierlessTimeout.stop();
-	isRecording = false;
-	keyButton->releaseKeyboard();
-	keyButton->setDown(false);
-	updateShortcutDisplay();
-	if (keySequence != oldKeySequence)
-		emit q->keySequenceChanged(keySequence);
+	updateSequence(false);
 }
 
 #if 0
Index: kkeysequencewidget.h
===================================================================
--- kkeysequencewidget.h	(revision 702289)
+++ kkeysequencewidget.h	(working copy)
@@ -79,11 +79,27 @@
 
 Q_SIGNALS:
 	/**
-	 * This signal is emitted when the current key sequence has changed, be it by user
-	 * input or programmatically.
+	 * This signal is emitted when the current key sequence has changed,
+	 * be it by user input or programmatically.
+	 *
+	 * @param seq The new key sequence, which is already set now.
 	 */
 	void keySequenceChanged(const QKeySequence &seq);
 
+	/**
+	 * This signal is also emitted when the current key sequence has changed,
+	 * be it by user input or programmatically.
+	 *
+	 * @param newSeq The new key sequence, which is already set now.
+	 * @param oldSeq The old key sequence, which was set before the key
+	 *               sequence changed.
+	 * @param programmatically Specifies wether the key sequence was changed
+	 *                         programmatically or by the user.
+	 */
+	void keySequenceChanged(const QKeySequence &newSeq,
+				const QKeySequence &oldSeq,
+				bool programmatically);
+
 public Q_SLOTS:
 	/**
 	 * Capture a shortcut from the keyboard. This call will only return once a key sequence

--Boundary-00=_+zfyGfXnFsYxAcM
Content-Type: text/x-diff;
  charset="us-ascii";
  name="kmail.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename="kmail.diff"

Index: kmfilterdlg.cpp
===================================================================
--- kmfilterdlg.cpp	(revision 702519)
+++ kmfilterdlg.cpp	(working copy)
@@ -526,25 +530,22 @@
   }
 }
 
-#ifdef __GNUC__
-#warning FIXME: Shortcut Handling broken
-// Shortcut handling is broken for custom templates, tags and filters.
-// The code assumes that the key sequence widget's shortcut is NOT already
-// set when this function is called, which is no longer the case with the
-// new KKeySequenceWidget.
-// This needs better support from KKeySequenceWidget. --tmcguire
-#endif
-void KMFilterDlg::slotCapturedShortcutChanged( const QKeySequence& ks )
+void KMFilterDlg::slotCapturedShortcutChanged( const QKeySequence& newSeq,
+                                               const QKeySequence& oldSeq,
+                                               bool programmatically )
 {
-  if ( !ks.isEmpty() && !( kmkernel->getKMMainWidget()->shortcutIsValid( ks ) ) ) {
+  if ( programmatically )
+    return;
+
+  if ( !newSeq.isEmpty() &&
+       !( kmkernel->getKMMainWidget()->shortcutIsValid( newSeq ) ) ) {
     QString msg( i18n( "The selected shortcut is already used, "
-          "please select a different one." ) );
+                       "please select a different one." ) );
     KMessageBox::sorry( this, msg );
-  } else {
-    mKeySeqWidget->setKeySequence( ks );
+    mKeySeqWidget->setKeySequence( oldSeq );
+  } else
     if ( mFilter )
-      mFilter->setShortcut( KShortcut(ks, QKeySequence()) );
-  }
+      mFilter->setShortcut( KShortcut(newSeq, QKeySequence()) );
 }
 
 void KMFilterDlg::slotConfigureToolbarButtonToggled( bool aChecked )
Index: kmfilterdlg.h
===================================================================
--- kmfilterdlg.h	(revision 702519)
+++ kmfilterdlg.h	(working copy)
@@ -364,7 +364,8 @@
   void slotApplicableAccountsChanged();
   void slotStopProcessingButtonToggled( bool aChecked );
   void slotConfigureShortcutButtonToggled( bool aChecked );
-  void slotCapturedShortcutChanged( const QKeySequence& );
+  void slotCapturedShortcutChanged( const QKeySequence&,
+                                    const QKeySequence&, bool );
   void slotConfigureToolbarButtonToggled( bool aChecked );
   void slotFilterActionIconChanged( const QString &icon );
   void slotReset();

--Boundary-00=_+zfyGfXnFsYxAcM--