Tooltip notification. Check for update fixes.

src/plugin/NotificationMessage.cpp

 #include <Windows.h>
 #include <CommCtrl.h>
 
 #include "NotificationMessage.h"
 
-NotificationMessage::NotificationMessage(HWND parent)
+NotificationMessage::NotificationMessage(HWND parent): parentWindow(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()
 {
   if (!commonControlsInitialized)
   {
@@ skipping 13 matching lines @@
   }
   toolTipWindow = CreateWindowEx(WS_EX_TOPMOST, TOOLTIPS_CLASS, NULL,
                                   TTS_NOPREFIX | TTS_BALLOON | TTS_CLOSE,
                                   0, 0,
                                   0, 0,
                                   parentWindow, NULL, NULL,
                                   NULL);
 
   SetWindowPos(toolTipWindow, HWND_TOPMOST,0, 0, 0, 0,
           SWP_NOMOVE | SWP_NOSIZE | SWP_NOACTIVATE);
-  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.

+  TOOLINFOW ti = {};
   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 = const_cast<LPWSTR>(message.c_str());    

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

Can you get rid of this trailing whitespace?

+  ti.lpszText = const_cast<LPWSTR>(message.c_str());
   GetClientRect(parentWindow, &ti.rect);
 
   LRESULT res = ::SendMessage(toolTipWindow, TTM_ADDTOOL, 0, (LPARAM)&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.

+  SetTextAndIcon(message, title, icon);
   res = ::SendMessage(toolTipWindow, TTM_TRACKACTIVATE, TRUE, (LPARAM)&ti);
 
   return true;
 }
 
 void NotificationMessage::Hide()
 {
   if (toolTipWindow != 0)
   {
     DestroyWindow(toolTipWindow);
     toolTipWindow = 0;
   }
 }
 
 void NotificationMessage::Move(short x, short y)
 {
   ::SendMessage(toolTipWindow, TTM_TRACKPOSITION, 0, MAKELONG(x, y));
   return;
 }
 
 bool NotificationMessage::SetTextAndIcon(std::wstring text, std::wstring title, int icon)
 {
-  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.

+  TOOLINFOW ti = {};
   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 = 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);
   return res == TRUE;
 }
 
 void NotificationMessage::SetParent(HWND parent)
 {
   parentWindow = parent;
 }
 
 bool NotificationMessage::IsVisible()
 {
   if (toolTipWindow == 0)
     return false;
   return IsWindowVisible(toolTipWindow);
 }