Jan Schiefer wrote:
It's already fully cleaned up and ready to go! I know other people hate my coding style... :)

Awww, it's not hate, you just need to be careful about how the code is laid out and what features you use. You'll need to fix this patch in a few places so here are some comments:


It's usually a good idea to submit the backend *first* and then the frontend, or better .. have them done all at once. Patches are supposed to be standalone, not stuff that'll only start working later (in the general case). For now I'd combine the two patches.

+    GROUPBOX        "Look at",IDC_FIND_GROUPBOX, 5, 25, 194, 46, BS_GROUPBOX
+    CHECKBOX       "Keys", IDC_CHECKBOX_KEYS, 10, 36, 60, 10

Try and keep alignment consistent, otherwise there's just extra visual noise for no good reason!


+LRESULT CALLBACK findedit_proc (HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM 
lParam) {
+if(uMsg != 193) { // if uMsg == 193, tmpLen is always 0!
+int tmpLen = SendMessage(hWnd, EM_LINELENGTH, 0, 0);
+if(tmpLen != searchStringLen) {
+if(tmpLen > 0)EnableWindow(hFindButton, TRUE);
+else EnableWindow(hFindButton, FALSE);
+searchStringLen = tmpLen;
+}
+}
+return CallWindowProc ((WNDPROC) prevWndProcEdit, hWnd, uMsg, wParam, lParam);
+}

I'm afraid that unless my mail client is playing up that type of code isn't acceptable. You need to use indenting! Also please try and space stuff out, eg rather than: if(tmpLen > 0)EnableWindow use if (tmpLen > 0) EnableWindow(....);


+
+INT_PTR CALLBACK searchingdlg_proc(HWND hWnd, UINT uMsg, WPARAM wParam,LPARAM 
lParam) {
+    switch(uMsg) {
+        case WM_INITDIALOG:
+       //SearchRegistryRecrusively("",HKEY_CLASSES_ROOT,TRUE);

No C++/C99 style comments unfortunately. Also please don't submit patches that have code commented out for no obvious reason.


+        break;
+       case WM_COMMAND:
+                   switch(LOWORD(wParam)) {
+           case IDCANCEL:
+           HeapFree(GetProcessHeap(),0,searchString);
+            EndDialog(hWnd, 0);
+            return(TRUE);
+           break;
+       }

Indenting is good, but indenting consistently is even better!

+ break;
+ }
+
+return(FALSE);
+}
+
+INT_PTR CALLBACK finddlg_proc(HWND hWnd, UINT uMsg, WPARAM wParam,LPARAM lParam) {
+
+ switch(uMsg) {
+ case WM_INITDIALOG:
+ hFindEdit = GetDlgItem(hWnd, IDC_FINDEDIT);
+ hFindButton = GetDlgItem(hWnd, IDOK);
+ prevWndProcEdit = SetWindowLongPtr (hFindEdit, GWLP_WNDPROC, (LONG_PTR) findedit_proc);
+ EnableWindow(hFindButton, FALSE); + CheckDlgButton( hWnd, IDC_CHECKBOX_KEYS, BST_CHECKED );
+ CheckDlgButton( hWnd, IDC_CHECKBOX_VALUES, BST_CHECKED );
+ CheckDlgButton( hWnd, IDC_CHECKBOX_DATA, BST_CHECKED );
+ return(TRUE);

I think you may be mixing tabs and spaces or something here. Also making return look like a function is rather non-standard for C, use it as a statement: return TRUE;


+       break;
+        case WM_COMMAND:
+            switch(LOWORD(wParam)) {
+               case IDOK:              
+               searchString = 
(LPTSTR)HeapAlloc(GetProcessHeap(),0,searchStringLen + 1);
+               GetWindowText(hFindEdit, searchString, searchStringLen + 1 );
+                                                               
+               searchForKeys = (IsDlgButtonChecked( hWnd, IDC_CHECKBOX_KEYS ) 
== BST_CHECKED);
+               searchForValues = (IsDlgButtonChecked( hWnd, 
IDC_CHECKBOX_VALUES ) == BST_CHECKED);
+               searchForData = (IsDlgButtonChecked( hWnd, IDC_CHECKBOX_DATA ) 
== BST_CHECKED);
+               searchMatchWholeString = (IsDlgButtonChecked( hWnd, 
IDC_CHECKBOX_MATCHSTRING ) == BST_CHECKED);

If you're going to fall through please put a comment like /* fall through */ otherwise it looks an awful lot like a mistake.


thanks -mike



Reply via email to