feat: add Windows GB18030 text encoding support and native cmd.exe process creation#131
Merged
Merged
Conversation
…ocess creation - Add shell.Command() wrapper for platform-conditional process creation - Windows: use SysProcAttr.CmdLine for cmd.exe to handle quoted paths - Non-Windows: delegate to exec.Command - Replace /s flag with /c in cmd.exe args to fix quoted-path handling - Add text encoding layer for Windows GB18030 (CP 936/54936) round-trip: - text_encoding.go: type and constants - text_decode.go / text_decode_windows.go: decode/encode functions - normalizeTextFileBytes/restoreTextFileBytes now preserve encoding - Consolidate decodeShellOutput on Windows to use shared decodeTextBytes - Update grepFile to normalize encoding (bufio.Scanner replaced with os.ReadFile + normalizeTextFileBytes) - Add readFile directory check with stable not_file error - Update all exec.Command(spec.Bin, spec.Args...) call sites to use shell.Command(spec) for consistency - Add comprehensive tests: GB18030 read/edit/search, cmd.exe quoted Chinese paths, directory error, grep empty/blank-line edge cases
Add an explicit exit statement after the & { ... } wrapper so
PowerShell propagates the native command's exit code instead of
always returning 0.
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
1 participant
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.
Summary
This PR adds comprehensive Windows text-encoding support and fixes cmd.exe process creation for non-ASCII paths.
Key Changes
Platform-conditional process creation ()
shell.Command(Spec)wrapper returns a platform-appropriate*exec.CmdSysProcAttr.CmdLineforKindCmdto bypass Go's quoting and handle paths with Chinese characters correctlyexec.Commandunchanged/sfrom cmd.exe args — it interfered with quoted paths containing non-ASCII charactersGB18030 text encoding round-trip (
internal/tools/text_*.go)textEncodingtype and constantstext_decode.go/text_decode_windows.go: on Windows with ANSI CP 936/54936, non-UTF8 file bytes are decoded from GB18030 on read and re-encoded on writenormalizeTextFileBytes/restoreTextFileBytesnow preserve encoding metadata through the round-tripdecodeShellOutputconsolidated to use the shareddecodeTextBytesFile tool improvements
readFile: added directory check returning a stablenot_fileerror (no OS-specific text)grepFile: switched frombufio.Scannertoos.ReadFile+normalizeTextFileBytesto support encoding-normalized searching on Windowsexec.Command(spec.Bin, spec.Args...)call sites updated toshell.Command(spec)Testing
cmd/dev/main.goto include all new Windows testsReview Notes
One minor area to watch:
command_windows.gopasses aCmdLinewithout argv[0] prefix — this is correct for cmd.exe (which parses flags directly) but should be documented for future maintainers.