notepad patches (search/replace, etc)

[Vitaliy Margolen]

Thursday, June 8, 2006, 6:40:33 PM, Anoni Moose wrote:
> This is my first patch to an open source project... if anyone has any comments/suggestions, please tell me. :)

> These patches add full search, search next, replace, and goto line functionality to notepad. Too bad I missed the 0.9.15 release!

> Changelog:

>     * programs/notepad/main.c, programs/notepad/dialog.c,
>       programs/notepad/main.h, programs/notepad/notepad_res.h,
>       programs/notepad/dialog.h:
>     notepad: Add/call functions to load/save globals settings (including font info) to registry.
>                 -Added full Search/Search Next functionality.
>                 -Added full Replace functionality.
>                 -Added full Goto Line functionality.
>                 -Load/Save to registry whether we want to wrap long lines or not.


Please one patch per email. You should combine all of your changes into one patch
if that's one logical change and can be applied by itself. Of course resultant code
should compile and work.

> @@ -638,6 +641,8 @@
>          Globals.hFont=CreateFontIndirect( &lf );
>          Globals.lfFont=lf;
>          SendMessage( Globals.hEdit, WM_SETFONT, (WPARAM)Globals.hFont, (LPARAM)TRUE );
> +     SETTINGS_SaveSettings();
> +
Please respect indentation. Don't just copy&paste stuff all over the file.

>  VOID DIALOG_Search(VOID)
>  {
> +
> +     if (Globals.find.hwndOwner == NULL) {
>          ZeroMemory(&Globals.find, sizeof(Globals.find));
> +     }
>          Globals.find.lStructSize      = sizeof(Globals.find);
>          Globals.find.hwndOwner        = Globals.hMainWnd;
Pretty much the same here: don't just insert stuff, indent it properly.
And please no extra blank lines nor needles curly brackets.

> +     if (Globals.replace.hwndOwner == NULL) {
> +       ZeroMemory(&Globals.replace, sizeof(Globals.replace));
> +     }
Respect indentation and style of the file you changing. In this case 4-spaces
not 2.

> +     Globals.hFindReplaceDlg = ReplaceText(&Globals.replace);
> +     assert(Globals.hFindReplaceDlg != 0);
> +}
Please don't use assert. Do a proper error checking instead.

> +     if (result == -1) {     /* text not found. */
> +       MessageBoxA(Globals.hEdit, "Cannot find text.", NULL, MB_OK | MB_ICONINFORMATION);
> +     } else {
You should put all the text into resource files.

> +WCHAR *GrabWindowTextW(HWND hWnd, DWORD *nbytes) {
> +
> +     static WCHAR *data = NULL;
> +     DWORD _nbytes = 0;
> +
> +     if (nbytes == NULL) nbytes = &_nbytes;
> +
> +     *nbytes = (*nbytes == 0 ? GetWindowTextLengthW(hWnd) + 1 : *nbytes);
> +     data = HeapAlloc(GetProcessHeap(), 0, (*nbytes) * sizeof(WCHAR));
> +     GetWindowTextW(hWnd, data, (*nbytes)+1);
> +     return data;
> +}
Why do you need static if you return allocated buffer?
nbytes is misleading - it should be nchars.
(*nbytes)+1 is incorrect. You allocated enough memory for *nbytes only.

> +int SearchText(HWND hWnd, LPFINDREPLACE find, int pos);
Please use windows types so they would work right on 64-bit platforms.

> +    "^R", CMD_REPLACE
Native notepad has it as ctrl+H.

Vitaliy Margolen