notepad patches (search/replace, etc)

[Anoni Moose]

Thanks for the input... I guess I've got a lot of fixing up to do. So, all
of the responses I have received, I guess mean that they need to be redone
before being accepted? If they don't get a response like "Accepted", they're
not accepted?

Maybe my strategy of making a bunch of changes and sending them all at once
wasn't too good of an idea. :)

I'll work on them and re-submit later.

On 6/8/06, Vitaliy Margolen <wine-devel at kievinfo.com> wrote:
>
> 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
>
>
>
>


-- 
- Anoni Moose
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://www.winehq.org/pipermail/wine-devel/attachments/20060609/0e0e2e3a/attachment.htm