Windows: fix command-line file opening and Unicode path support (#30)#31
Open
tjfunction412 wants to merge 2 commits into
Open
Windows: fix command-line file opening and Unicode path support (#30)#31tjfunction412 wants to merge 2 commits into
tjfunction412 wants to merge 2 commits into
Conversation
…ecruncher#30) Two related bugs prevented Windows users from opening CSV files via file association or command line. (1) WinMain in main.cpp received lpCmdLine but never parsed it. The non-Windows main(argc, argv) had an argv loop calling CsvApplication::droppedFileCB, but it was excluded from _WIN64 builds via #ifndef. Files passed by Explorer or PowerShell were silently dropped and an empty Empty 1 window appeared. Fixed by parsing the wide command line with CommandLineToArgvW (which preserves Unicode characters that lpCmdLine and __argv would lose to ANSI codepage conversion) and routing each argument through Helper::ws_to_utf8 and droppedFileCB, matching the existing non-Windows code path. (2) Helper::ws_to_utf8 and Helper::utf8_to_ws were declared in helper.hh but never implemented. Added Windows implementations using WideCharToMultiByte / MultiByteToWideChar with CP_UTF8. (3) Tablecruncher file I/O used std::ifstream::open(const char*) and std::ofstream(const char*, ...) with UTF-8 std::string paths. MSVC interprets these narrow paths as the active ANSI codepage, not UTF-8, so any filename outside that codepage failed to load even when the new argv parser delivered it correctly. This broke Unicode filenames passed via Explorer. Added Helper::openInputStream / openOutputStream wrappers that transcode UTF-8 paths to wide on Windows and use the MSVC wstring overloads, while keeping the narrow path on POSIX. Routed all 5 file-open call sites through the new helpers. Fixed Helper::getFileSize similarly (stat -> _wstat64). Replaced std::filesystem::path(std::string) calls in the recent-files loader and menu-label paths with std::filesystem::u8path, which correctly interprets the input as UTF-8 across platforms. Tested manually with ASCII paths, paths with spaces, Latin-1 supplement diacritics, Cyrillic, and CJK filenames; via command line, multi-file launch, file association double-click, and File > Save As round-trip.
Previously the build relied on a post-build Resource Hacker step (scripts/buildwin.bat) to inject the application icon into the built Tablecruncher.exe. This added an external-tool dependency and meant building from source without the Inno Setup release workflow produced an icon-less binary. Added a minimal assets/windows/app_icon.rc pointing to the existing app_icon.ico, and wired it into CMakeLists.txt via enable_language(RC) and an extra source on Windows. MSVC's rc.exe now embeds the icon during the normal build. The Resource Hacker step in buildwin.bat is left in place for now but is redundant for icon embedding and could be removed in a follow-up.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #30.
This PR addresses two related Windows bugs and bundles a small build improvement.
Fix 1 — Command-line file opening (issue #30)
WinMaininsrc/main.cppreceivedlpCmdLinebut never parsed it. The non-Windowsmain(argc, argv)had an argv loop callingCsvApplication::droppedFileCB, but it was excluded from_WIN64builds via#ifndef. As a result, files passed via Windows Explorer file association orTablecruncher.exe path/to/file.csvwere silently dropped and an empty "Empty 1" window opened instead.Fixed by parsing the wide command line with
CommandLineToArgvWand routing each argument throughHelper::ws_to_utf8→CsvApplication::droppedFileCB, matching the existing non-Windows behavior.CommandLineToArgvWis the right entry point here:lpCmdLineand__argvare ANSI and would lose Unicode characters to the active codepage.Fix 2 — Unicode path support throughout file I/O
Helper::ws_to_utf8andHelper::utf8_to_wswere declared inhelper.hhbut never implemented. Added Windows implementations usingWideCharToMultiByte/MultiByteToWideCharwithCP_UTF8.Tablecruncher's file I/O used
std::ifstream::open(const char*)andstd::ofstream(const char*, ...)with UTF-8std::stringpaths. MSVC interprets these narrow paths as the active ANSI codepage, not UTF-8, so any filename outside that codepage failed to load even after Fix 1 delivered it correctly. This broke Unicode filenames passed via Explorer (Cyrillic, CJK, many Latin-1 supplement characters).Added
Helper::openInputStream/Helper::openOutputStreamwrappers that transcode UTF-8 paths to wide on Windows and use the MSVCwstringoverloads, while keeping the narrow path on POSIX. Routed all five file-open call sites through the new helpers:csvwindow.cppload pathcsvapplication.cpptwo existence checks (split CSV, save dialog)csvtable.cpptwo save paths (save CSV, export JSON)Fixed
Helper::getFileSizesimilarly (stat→_wstat64on Windows). Replacedstd::filesystem::path(std::string)calls in the recent-files loader and the menu-label code withstd::filesystem::u8path, which correctly interprets the input as UTF-8 across platforms.Bonus — Embed icon at build time
The build previously relied on a post-build Resource Hacker step (
scripts/buildwin.bat) to inject the application icon. This makes building from source without the Inno Setup release workflow produce an icon-less binary. Added a minimalassets/windows/app_icon.rcand wired it into CMake viaenable_language(RC), so MSVC'src.exeembeds the icon during the normal build.The Resource Hacker call in
buildwin.batis left in place to avoid scope creep but is now redundant for icon embedding.Testing
Built with Visual Studio Build Tools 2026 (MSVC 14.51, x64) and FLTK 1.4.5 on Windows 10. Manually verified:
a.csv b.csv c.csv) — first file populates the initial window, subsequent files open new windowsCompatibility
Behavior on macOS and Linux is unchanged. All Windows-specific code is guarded by
#ifdef _WIN64(matching the existing pattern in the codebase). The new helper functions have non-Windows paths that preserve the prior narrow-string behavior.