hashlink
hashlink copied to clipboard
HaxeFoundation
Allow multiple file selection in ui_choose_file
This is one of two PRs (The other in haxe for ui.hx) to add support for multiple selection in hl.UI.chooseFile. The haxe side PR can be found here: https://github.com/HaxeFoundation/haxe/pull/10788
Both do weird things for dumb windows API reasons, so I'll explain my sins in advance. Documentation for this API can be found here: https://docs.microsoft.com/en-us/windows/win32/api/commdlg/ns-commdlg-openfilenamea (We use the widechar version, but otherwise the call pattern is identical).
To enable multi select, we really only need to set a flag. Since we just pass a simple Dynamic across the fence for this, I just key off a new field and set the related flag. Note we also have to set OFN_EXPLORER else you get the win 3.x era version of this dialogue... which is magical in its own right.
The elephant in the room though is the larger buffer, and the fact that we return the entire thing. The buffer size increase is because this API doesn't really have any mechanism for letting you ask for how many bytes you need; hence we should always oversize. And the size chosen is simply due to that being the largest buffer you can even send this API. We'll need to use a newer API if we want anything more sophisticated.
The reason we pass the whole buffer, instead of the old behavior of just copying the length, is because the return format is null terminated. So wcslen will only ever return the path. The haxe commit has the untangling code for converting this data into haxe strings. This code isn't performance critical, so the extra copy shouldn't be a huge deal, and String.fromUCS2 doesn't rely on buffer size.
Hi, Looks fine, the only issue I have is that the windows specific implementation (in particular the 2048 buffer size) is visible in the Haxe code (idx < 2048). If we can get rid of that so that another platform API can return a valid buffer with no 2048 size limit I think we're good and it can be merged. Best, Nicolas
Pushed an update to both PRs.
That line was originally just a safety valve during testing, but removing and testing revealed a rather exciting bug: nMaxFile in the windows API is a character limit, not a buffer size! Since we're using the W version, the old value was actually a potential crash, and we just never hit it because of the windows path limit.
I fixed this and reverted some other unnecessary changes I made in the process. Buffer sizing is now accounting for wchar size, and appears to behave correctly when fed too many files (returns null)
The final hl_copy_bytes value seems incorrect. maybe should be sizeof(outputFile) ?
I wonder if this helps the File.browse() issue I ran into https://github.com/HeapsIO/heaps/issues/783
