Tooltip notification. Check for update fixes.

src/plugin/NotificationMessage.cpp

Index: src/plugin/NotificationMessage.cpp
===================================================================
--- a/src/plugin/NotificationMessage.cpp
+++ b/src/plugin/NotificationMessage.cpp
@@ -1,28 +1,24 @@
-#include <Windows.h>
-#include <CommCtrl.h>
+#include <Windows.h>
+#include <CommCtrl.h>
 
 #include "NotificationMessage.h"
 
-NotificationMessage::NotificationMessage()
+NotificationMessage::NotificationMessage(HWND parent)
+{ 
+  parentWindow = parent;

[Felix Dahlke, 2013/10/01 12:59:53]

Would rather do this in the initialiser list.

+  toolTipWindow = 0;
+  InitializeCommonControls();
+};
+
+bool NotificationMessage::commonControlsInitialized(false);
+
+void NotificationMessage::InitializeCommonControls()
 {
-  CommonControlsInitialize();
-}
-
-NotificationMessage::NotificationMessage(HWND parent)
-{ 
-  parentWindow = parent;
-  CommonControlsInitialize();
-};
-
-bool NotificationMessage::commonControlsInitialized(false);
-
-void NotificationMessage::CommonControlsInitialize()
-{
-  if (!commonControlsInitialized)
-  {
-    INITCOMMONCONTROLSEX commControls;
-    commControls.dwSize = sizeof(INITCOMMONCONTROLSEX);
-    commControls.dwICC = ICC_BAR_CLASSES;
+  if (!commonControlsInitialized)
+  {
+    INITCOMMONCONTROLSEX commControls;
+    commControls.dwSize = sizeof(INITCOMMONCONTROLSEX);
+    commControls.dwICC = ICC_BAR_CLASSES;
     InitCommonControlsEx(&commControls);
     commonControlsInitialized = true;
   }
@@ -30,6 +26,10 @@
 
 bool NotificationMessage::Show(std::wstring message, std::wstring title, int icon)
 {
+  if (toolTipWindow != 0)
+  {
+    Hide();
+  }
   toolTipWindow = CreateWindowEx(WS_EX_TOPMOST, TOOLTIPS_CLASS, NULL,
                                   TTS_NOPREFIX | TTS_BALLOON | TTS_CLOSE,
                                   0, 0,
@@ -39,52 +39,54 @@
 
   SetWindowPos(toolTipWindow, HWND_TOPMOST,0, 0, 0, 0,
           SWP_NOMOVE | SWP_NOSIZE | SWP_NOACTIVATE);
-  TOOLINFO ti;
-	ti.cbSize = sizeof(TOOLINFO);
+  TOOLINFOW ti;

[Felix Dahlke, 2013/10/01 12:59:53]

Would rather have all members set to zeroes, just in case we don't set them all
below.

We typically do that, either by using ZeroMemory/memset or by default
initialising (TOOLINFOW ti = {}). I prefer the latter.

+  ti.cbSize = sizeof(TOOLINFOW);
   ti.uFlags = TTF_IDISHWND | TTF_TRACK | TTF_TRANSPARENT;
-	ti.hwnd = toolTipWindow;
-	ti.hinst = NULL;
-	ti.uId = (UINT_PTR)parentWindow;
-	ti.lpszText = (LPWSTR)message.c_str();		
+  ti.hwnd = toolTipWindow;
+  ti.hinst = NULL;
+  ti.uId = (UINT_PTR)parentWindow;
+  ti.lpszText = const_cast<LPWSTR>(message.c_str());    

[Felix Dahlke, 2013/10/01 12:59:53]

Can you get rid of this trailing whitespace?

   GetClientRect(parentWindow, &ti.rect);
 
-	LRESULT res = ::SendMessage(toolTipWindow, TTM_ADDTOOL, 0, (LPARAM) (LPTOOLINFO) &ti);
-
-  RECT rect;
-  GetWindowRect(parentWindow, &rect);
-  Move(rect.left + (rect.right - rect.left) / 2, rect.top + (rect.bottom - rect.top) / 2); 
+  LRESULT res = ::SendMessage(toolTipWindow, TTM_ADDTOOL, 0, (LPARAM)&ti);
 
-	res = ::SendMessage(toolTipWindow, TTM_SETTITLE, icon, (LPARAM)title.c_str());
-	res = ::SendMessage(toolTipWindow, TTM_TRACKACTIVATE, TRUE, (LPARAM)(LPTOOLINFO) &ti);
+  RECT rect;
+  GetWindowRect(parentWindow, &rect);
+  Move(rect.left + (rect.right - rect.left) / 2, rect.top + (rect.bottom - rect.top) / 2); 
+
+  SetTextAndIcon(message.c_str(), title.c_str(), icon);

[Wladimir Palant, 2013/09/25 15:12:48]

Calling c_str() is unnecessary here. Also, this isn't quite what I meant - but
seeing that the messages aren't really identical in the two cases, not setting
ti.flags and ti.rect in SetTextAndIcon() should do to reduce code duplication.

+  res = ::SendMessage(toolTipWindow, TTM_TRACKACTIVATE, TRUE, (LPARAM)&ti);
 
   return true;
 }
 
-bool NotificationMessage::Hide()
+void NotificationMessage::Hide()
 {
-  DestroyWindow(toolTipWindow);
-  toolTipWindow = 0;
-  return true;
+  if (toolTipWindow != 0)
+  {
+    DestroyWindow(toolTipWindow);
+    toolTipWindow = 0;
+  }
 }
 
 void NotificationMessage::Move(short x, short y)
 {
-	::SendMessage(toolTipWindow, TTM_TRACKPOSITION, 0, (LPARAM)(LPTOOLINFO)MAKELONG(x, y));
+  ::SendMessage(toolTipWindow, TTM_TRACKPOSITION, 0, MAKELONG(x, y));
   return;
 }
 
 bool NotificationMessage::SetTextAndIcon(std::wstring text, std::wstring title, int icon)
 {
-  TOOLINFO ti;
-	ti.cbSize = sizeof(TOOLINFO);
+  TOOLINFOW ti;

[Felix Dahlke, 2013/10/01 12:59:53]

As above, would prefer to default initialise this to avoid accidental garbage
values for fields we don't set.

+  ti.cbSize = sizeof(TOOLINFOW);
   ti.uFlags = TTF_IDISHWND | TTF_TRACK | TTF_TRANSPARENT;
-	ti.hwnd = toolTipWindow;
-	ti.hinst = NULL;
-	ti.uId = (UINT_PTR)parentWindow;
-	ti.lpszText = (LPWSTR)text.c_str();		
+  ti.hwnd = toolTipWindow; 
+  ti.hinst = NULL;
+  ti.uId = (UINT_PTR)parentWindow;
+  ti.lpszText = const_cast<LPWSTR>(text.c_str());   
   GetClientRect(parentWindow, &ti.rect);
-	LRESULT res = ::SendMessage(toolTipWindow, TTM_SETTITLE, icon, (LPARAM)title.c_str());
-	res = ::SendMessage(toolTipWindow, TTM_UPDATETIPTEXT, 0, (LPARAM)&ti);
+  LRESULT res = ::SendMessage(toolTipWindow, TTM_SETTITLE, icon, (LPARAM)title.c_str());
+  res = ::SendMessage(toolTipWindow, TTM_UPDATETIPTEXT, 0, (LPARAM)&ti);
   return res == TRUE;
 }
 
@@ -92,6 +94,7 @@
 {
   parentWindow = parent;
 }
+
 bool NotificationMessage::IsVisible()
 {
   if (toolTipWindow == 0)