September 2022 - Wine-gitlab

[PATCH v8 0/1] MR766: advapi32/test: Add tests for RegLoadAppKey.
by Santino Mazza (＠tati1454) 07 Sep '22

07 Sep '22

-- v8: advapi32/tests: Add tests for RegLoadAppKey. https://gitlab.winehq.org/wine/wine/-/merge_requests/766

2 1

[PATCH v7 0/1] MR766: advapi32/test: Add tests for RegLoadAppKey.
by Santino Mazza (＠tati1454) 07 Sep '22

07 Sep '22

-- v7: advapi32/tests: Add tests for RegLoadAppKey. https://gitlab.winehq.org/wine/wine/-/merge_requests/766

3 2

Re: [PATCH v4 0/2] MR787: notepad: Add status bar and goto line dialog
by Huw Davies (＠huw) 07 Sep '22

07 Sep '22

Huw Davies (@huw) commented about programs/notepad/notepad.rc: > MENUITEM "&Search next\tF3", CMD_SEARCH_NEXT > MENUITEM "&Replace...\tCtrl+H", CMD_REPLACE > } > +POPUP "&View" > +{ > + MENUITEM "&Status Bar", CMD_SBAR > + } Another trailing space after `"&View"` and a space before the `}`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/787#note_7878

1 0

Re: [PATCH v4 0/2] MR787: notepad: Add status bar and goto line dialog
by Huw Davies (＠huw) 07 Sep '22

07 Sep '22

Huw Davies (@huw) commented about programs/notepad/main.c: > + > +VOID UpdateStatusBar(VOID) > +{ > + int currentLine; > + int currentCol = -1; > + wchar_t statusTxt[25]; > + int lineIndex; > + DWORD selStart; > + DWORD selEnd; > + > + SendMessageW(Globals.hEdit,EM_GETSEL , (WPARAM)&selStart,(LPARAM)&selEnd); > + if(selStart == selEnd) > + Globals.trackedSel = selStart; > + if(selStart < Globals.trackedSel) > + currentCol = selStart; > + else And this has a trailing space after the `else`. There are other instances below as well. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/787#note_7877

1 0

Re: [PATCH v4 0/2] MR787: notepad: Add status bar and goto line dialog
by Huw Davies (＠huw) 07 Sep '22

07 Sep '22

Huw Davies (@huw) commented about programs/notepad/main.c: > + CheckMenuItem(GetMenu(Globals.hMainWnd),CMD_SBAR, > + MF_BYCOMMAND | (Globals.bStatusBar ? MF_CHECKED : MF_UNCHECKED)); > + GetClientRect(Globals.hMainWnd, &rc); > + ShowWindow(Globals.hStatusBar,Globals.bStatusBar); > + SendMessageW(Globals.hMainWnd,WM_SIZE,0,MAKELPARAM(rc.right,rc.bottom)); > +} > + > +VOID UpdateStatusBar(VOID) > +{ > + int currentLine; > + int currentCol = -1; > + wchar_t statusTxt[25]; > + int lineIndex; > + DWORD selStart; > + DWORD selEnd; > + This line contains 4 spaces. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/787#note_7876

1 0

Re: [PATCH v4 0/2] MR787: notepad: Add status bar and goto line dialog
by Huw Davies (＠huw) 07 Sep '22

07 Sep '22

Huw Davies (@huw) commented about programs/notepad/main.c: > } > } > > +LRESULT CALLBACK EDIT_CallBackProc(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam) > +{ > + switch (msg) > + { > + case WM_KEYDOWN: > + UpdateStatusBar(); > + break; > + case WM_KEYUP: > + UpdateStatusBar(); > + break; > + case WM_MBUTTONDOWN: > + UpdateStatusBar(); > + break; This could all be simplifed: ```c case WM_KEYDOWN: case WM_KEYUP: case WM_MBUTTONDOWN: ... UpdateStatusBar(); break; ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/787#note_7875

1 0

Re: [PATCH v4 0/2] MR787: notepad: Add status bar and goto line dialog
by Huw Davies (＠huw) 07 Sep '22

07 Sep '22

Huw Davies (@huw) commented about programs/notepad/main.c: > return dpi; > } > > +VOID ToggleStatusBar(VOID) > +{ > + RECT rc; > + > + Globals.bStatusBar = !Globals.bStatusBar; > + CheckMenuItem(GetMenu(Globals.hMainWnd),CMD_SBAR, > + MF_BYCOMMAND | (Globals.bStatusBar ? MF_CHECKED : MF_UNCHECKED)); > + GetClientRect(Globals.hMainWnd, &rc); > + ShowWindow(Globals.hStatusBar,Globals.bStatusBar); > + SendMessageW(Globals.hMainWnd,WM_SIZE,0,MAKELPARAM(rc.right,rc.bottom)); Again, call your newly added helper function instead of sending `WM_SIZE`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/787#note_7874

1 0

Re: [PATCH v4 0/2] MR787: notepad: Add status bar and goto line dialog
by Huw Davies (＠huw) 07 Sep '22

07 Sep '22

Huw Davies (@huw) commented about programs/notepad/main.c: > return dpi; > } > > +VOID ToggleStatusBar(VOID) `static` (as the function isn't used outside the file). Also, and I know it not the case in this file, but let's use `void` rather than `VOID`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/787#note_7873

1 0

Re: [PATCH v4 0/2] MR787: notepad: Add status bar and goto line dialog
by Huw Davies (＠huw) 07 Sep '22

07 Sep '22

Huw Davies (@huw) commented about programs/notepad/dialog.c: > Globals.bWrapLongLines = !Globals.bWrapLongLines; > CheckMenuItem(GetMenu(Globals.hMainWnd), CMD_WRAP, > MF_BYCOMMAND | (Globals.bWrapLongLines ? MF_CHECKED : MF_UNCHECKED)); > + SendMessageW(Globals.hMainWnd,WM_SIZE,0,MAKELPARAM(rc.right,rc.bottom)); This looks hacky. You probably want to move the code that you're adding to the main window's WM_SIZE handler to a helper function and call that directly from here. Also, and this applies throughout, please leave a space after each comma. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/787#note_7872

1 0

Re: [PATCH v4 0/2] MR787: notepad: Add status bar and goto line dialog
by Huw Davies (＠huw) 07 Sep '22

07 Sep '22

Generally I think this MR should be split into two separate ones. That'll give us a chance to get the status bar work in before we worry about the goto dialog box. So, just concentrating on the status bar commit, I'd start off by splitting that into two commits. First add the code that creates and displays the status bar (it doesn't need to display any text at this point). Then add a second commit that updates the status bar with line / col info. As @besentv mentioned, the commit messages are still not quite there - we terminate the subject with a period. I'll comment on some of the code below. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/787#note_7869

1 0