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.
-
Joe Souza (＠JoeS209)