Re: notepad patches (search/replace, etc)
On Sat, Jun 10, 2006 at 09:52:50AM -0600, Vitaliy Margolen wrote: > Friday, June 9, 2006, 11:49:44 AM, Anoni Moose wrote: > > Let's try again... I apologize for the bad indentation. It's indented fine > > in > > emacs, which I use; however, when I use "diff -u file1 file2", it gives me > > the > > entire file as a patch. Therefore, I must use "diff -uwB". Emacs must be > > screwing it up... Your editor adds DOS linefeeds for some reason. Ciao, Marcus
Re: notepad patches (search/replace, etc)
Friday, June 9, 2006, 11:49:44 AM, Anoni Moose wrote: > Let's try again... I apologize for the bad indentation. It's indented fine in > emacs, which I use; however, when I use "diff -u file1 file2", it gives me the > entire file as a patch. Therefore, I must use "diff -uwB". Emacs must be > screwing it up... That will not work. Your patch look really bad with all types of indentation. You need to use something that doesn't autoindent. Also please do not mix spaces and tabs - you have that on each line and that's agreed NO-NO. You either use tabs (8 spaces) or you use all spaces. But not tab & spaces on the same line. Vitaliy.
Re: notepad patches (search/replace, etc)
Let's try again... I apologize for the bad indentation. It's indented fine in emacs, which I use; however, when I use "diff -u file1 file2", it gives me the entire file as a patch. Therefore, I must use "diff -uwB". Emacs must be screwing it up... ChangeLog:* programs/notepad/main.c, programs/notepad/main.h, programs/notepad/dialog.c, programs/notepad/dialog.h, programs/notepad/notepad_res.h, programs/notepad/En.rc, programs/notepad/rsrc.rc: notepad: Add search, replace, goto line functionality.-- - Anoni Moose notepad_srg.diff Description: Binary data
Re: notepad patches (search/replace, etc)
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 <[EMAIL PROTECTED]> 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 patchif that's one logical change and can be applied by itself. Of course resultant codeshould 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 = "">> + DWORD _nbytes = 0;> +> + if (nbytes == NULL) nbytes = &_nbytes;> +
> + *nbytes = (*nbytes == 0 ? GetWindowTextLengthW(hWnd) + 1 : *nbytes);> + data = "" 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_REPLACENative notepad has it as ctrl+H.
Vitaliy Margolen-- - Anoni Moose
Re: notepad patches (search/replace, etc)
Christoph Frick <[EMAIL PROTECTED]> writes: > what is the difference between anonymous submissions, that we can spot > and which we don't? maybe you are a fake ;) Of course there's no real difference, but collaboration is based on trust, and it's hard to trust people who don't even want to give out their name. Obviously you can give me a false name, but I'll figure it out eventually, and then trust you even less than an anonymous submitter so there's really no point in trying to cheat. If you have good reasons for wanting to remain anonymous, please discuss it with me in private and we can arrange something; but otherwise please use your real name. -- Alexandre Julliard [EMAIL PROTECTED]
Re: notepad patches (search/replace, etc)
Hi, On Fri, Jun 09, 2006 at 10:52:58AM +0200, Christoph Frick wrote: > On Fri, Jun 09, 2006 at 10:08:11AM +0200, Andreas Mohr wrote: > > > Or, IOW, do we have any guidelines about "Anoni Moose" submissions to > > our project? Are they ok, not ok, ok? Loves me, loves me not, ... > > what is the difference between anonymous submissions, that we can spot > and which we don't? maybe you are a fake ;) Darn, I knew fully well that exactly this objection would arise... ;) Me, I've been created as an IRC bot happily typing away for about a decade without anyone noticing... Oh, that's not true in fact, people do know that I'm real, since someone has been showing up at Codeweavers and various wineconfs pretending to be me ;) So I guess it's fully a non-issue after all since the internet is a completely anonymous space anyway and patches can only be judged by their quality, not by their origin. Sorry for the noise! Andreas
Re: notepad patches (search/replace, etc)
On Fri, Jun 09, 2006 at 10:08:11AM +0200, Andreas Mohr wrote: > Or, IOW, do we have any guidelines about "Anoni Moose" submissions to > our project? Are they ok, not ok, ok? Loves me, loves me not, ... what is the difference between anonymous submissions, that we can spot and which we don't? maybe you are a fake ;) -- cu pgpPl77O0dqS0.pgp Description: PGP signature
Re: notepad patches (search/replace, etc)
Hi, On Thu, Jun 08, 2006 at 08:40:33PM -0400, Anoni Moose wrote: > This is my first patch to an open source project... if anyone has any > comments/suggestions, please tell me. :) I can't help but say that your parents must have been giving you a rather "funny" name... Or, IOW, do we have any guidelines about "Anoni Moose" submissions to our project? Are they ok, not ok, ok? Loves me, loves me not, ... Anyway, thanks for a very nice collection of patches! Andreas Mohr
Re: notepad patches (search/replace, etc)
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
