April 2025 - Wine-gitlab - list.winehq.org

Re: [PATCH v24 0/1] MR7843: cmd: Implement tab completion for command line entry.
by Joe Souza (＠JoeS209) 26 Apr '25

26 Apr '25

On Sat Apr 26 15:12:31 2025 +0000, eric pouech wrote: > thanks > a couple of high level comments: > * I don't think having a separate source file just for \~400 LoC is > worth it > * WCMD_ReadConsole is way too big as a function and need to be split in > smaller pieces. > * you should also be using the command line parsing functions instead of > rewriting them (eg WCMD_parameter to loop against the various parameters) > from quick testing: > * when looping around files sometimes filename is inserted in quotes, > while it shouldn't. Native seems to insert quotes only when filename > contains specific characters (spaces...), but to also remove them if > next file doesn't contain any... > * using "cd c:\\Program Files"\<tab\>\<backspace\>\\\<tab\> doesn't > behave as native... native circles in the directory content, current MR > only adds a double quote > * got a crash when playing with MR (perhaps one of the buffer overflow > stated above, perhaps not) > * when the width of the console is smaller than the full line input (eg > spread at least on two lines), circling gives some strange result > (cursor and display when from bottom of screen buffer back to top, will > previous content not shifted) > likely the current appoach of just having a handle to findfirst/findnext > will not fit all the (long term) goals. at some point, we want to > support shift-tab which circles backwards in the matching files. and I > fail to see what using FindFirstFileEx brings as value, as you filter > against directory when needed (I'm not asking for supporting shift-tab > right now, but rather to put in place a structure were it wil fit simply > when we implement it) > support for cd/rmdir/ and friends should be added in a second patch > I didn't go further in the review given the amount a remarks and > problems stated above. I'll restart when they are solved > (will look in // to the ESC issue raised) In the latest changes I have already split out much code from WCMD_ReadConsole into subfunctions. You're saying it's still too long? I suppose that I could split out the code for the individual case statements into separate handler functions, but the code to handle Tab is the bulk of that function. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7843#note_101954

1 0

Re: [PATCH v24 0/1] MR7843: cmd: Implement tab completion for command line entry.
by Joe Souza (＠JoeS209) 26 Apr '25

26 Apr '25

On Sat Apr 26 15:12:31 2025 +0000, eric pouech wrote: > that's not sufficient, you have also to compare that inputbuffer up to > current pos hasn't changed since previous call > (eg directory with za, za2 and zb zb2 files... circle twice from "za", > replace "za" by "zb" and circle with tab...) I missed that case. I tested C:\win<tab> and C:\pro<tab> but resulting paths are different lengths. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7843#note_101953

1 0

Re: [PATCH v24 0/1] MR7843: cmd: Implement tab completion for command line entry.
by Joe Souza (＠JoeS209) 26 Apr '25

26 Apr '25

On Sat Apr 26 15:12:31 2025 +0000, eric pouech wrote: > looks to be that curPos can be \> \*numRead, hence reading garbage in > that case Line 312 terminates the buffer at *numRead, line 314 sets len to *numRead, and loop at 320 stops when !(curPos < len). Worst case is if curPos == len then we are checking the null terminator. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7843#note_101952

1 0

[PATCH v2 0/1] MR7888: wbemprox: Implement Win32_CacheMemory table.
by Paul Gofman (＠gofman) 26 Apr '25

26 Apr '25

-- v2: wbemprox: Implement Win32_CacheMemory table. https://gitlab.winehq.org/wine/wine/-/merge_requests/7888

3 3

Re: [PATCH v24 0/1] MR7843: cmd: Implement tab completion for command line entry.
by Joe Souza (＠JoeS209) 26 Apr '25

26 Apr '25

On Sat Apr 26 17:10:07 2025 +0000, Joe Souza wrote: > Paths need to be quoted if they are long file names or they contain > spaces, hence the check for findData.cAlternateFileName. To do what you're asking here we would need to track whether each level in the path requires quotes, not just the current filename. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7843#note_101950

1 0

Re: [PATCH v24 0/1] MR7843: cmd: Implement tab completion for command line entry.
by Joe Souza (＠JoeS209) 26 Apr '25

26 Apr '25

On Sat Apr 26 15:12:32 2025 +0000, eric pouech wrote: > I think quote handling is not done as it should > from what I see on native: quotes are added when generated word does > contain characters that would need to be escaped (eg ' ', but didn't > check all the others) > need quote is an attribute of current filename (ie they are removed when > the new (partial) matching file no longer needs them) ; so quotes > shouldn't be part of the SEARCH_CONTEXT, but only used for display > (basically: go to beg of word; if needs quotes, print quote; print > (partial) word; print ending quotes (if needed) Paths need to be quoted if they are long file names or they contain spaces, hence the check for findData.cAlternateFileName. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7843#note_101949

1 0

[PATCH 0/2] MR7884: odbc32: Fix some NULL pointer issues
by Stefan Dösinger 26 Apr '25

26 Apr '25

I ran into this trying to use the Mac-Native MS SQL Server ODBC driver -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7884

2 3

[PATCH 0/1] MR7912: appwiz.cpl: Report addon download failures.
by Esme Povirk (＠madewokherd) 26 Apr '25

26 Apr '25

This relates to https://bugs.winehq.org/show_bug.cgi?id=58160 but doesn't fix it, just reports the error instead of stalling. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7912

2 1

[PATCH 0/1] MR7911: odbc32: Fix replicating unixODBC data sources.
by Hans Leidekker (＠hans) 26 Apr '25

26 Apr '25

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58140 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7911

2 1

Re: [PATCH v21 0/1] MR7843: cmd: Implement tab completion for command line entry.
by eric pouech (＠epo) 26 Apr '25

26 Apr '25

eric pouech (@epo) commented about programs/cmd/lineedit.c: > + } > + oldCurPos = curPos; > + curPos = 0; > + while (curPos < len && > + inputBuffer[curPos] != L'\t' && > + inputBuffer[curPos] != L'\x1B' > + ) { > + > + curPos++; > + } > + /* curPos is often numRead - 1, but not always, as in the case where history is retrieved > + * and then user backspaces to somewhere mid-string and then hits Tab. > + */ > + TRACE("numRead: %lu, curPos: %u\n", *numRead, curPos); > + > + switch (inputBuffer[curPos]) { looks to be that curPos can be \> \*numRead, hence reading garbage in that case -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7843#note_101933

1 0