Re: [PATCH] Add search functionality to WINE regedit

2005-03-08 Thread Mike Hearn
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.

+GROUPBOXLook 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


Re: [PATCH] Add search functionality to WINE regedit

2005-03-08 Thread Oliver Stieber

--- Mike Hearn [EMAIL PROTECTED] wrote:
 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:
 
Also..
   if(uMsg != 193) { // if uMsg == 193, tmpLen is
always 0!

no C++ style comments. try to use /* comment */

I use this a file containing 

^[[:space:]]+#
//
/\*[^[:space:]]\w
\w[^[:space:]]\*/
if\(
)[[:alpha:]]

with 

find -mtime -10 -iregex .*\.[c | h] -exec egrep 
--file=filewithchecks {}  -n \; -print

add -wError and -Werror-implicit-function-declaration 
and -Wdeclaration-after-statement for gcc = 3.4 to
cflags in your Makefile. 

I'll post a patch so that the whole of wine can be
built with -wError on gcc =3.3 soon, 

for the brave use:

(long long) for shift errors  32 is greater than


programs/winehlp uses %x so the Makefile needs
-Wno-format-y2k in it's CFLAGS

and add
register int src char * in libs/port/memmove.c



Send instant messages to your online friends http://uk.messenger.yahoo.com