Skip to content

feat: Add support for "UniGB-UCS2-H" encoding. - #56

Merged
ledongthuc merged 4 commits into
ledongthuc:masterfrom
kvii:pr_ucs2
Sep 7, 2026
Merged

ledongthuc merged 4 commits into
ledongthuc:masterfrom
kvii:pr_ucs2

Conversation

@kvii

@kvii kvii commented Oct 8, 2025

Copy link
Copy Markdown

Fix #55

@kvii kvii changed the title feat: Add support for "UniGB-UCS2-H" encoding. #55 feat: Add support for "UniGB-UCS2-H" encoding. Oct 8, 2025
@ledongthuc

Copy link
Copy Markdown
Owner

Thanks for the PR — this is a clean fix for #55. The approach is correct: UniGB-UCS2-H uses big-endian UCS-2 source codes (the content-stream code is the Unicode code point, and the CMap maps it to a CID only for glyph selection), so decoding as UTF-16 is the right strategy. A few small cleanups before merge:

1. Reuse the existing utf16Decode helper instead of binary.Read

text.go already has the identical big-endian decode:

func utf16Decode(s string) string {
    var u []uint16
    for i := 0; i < len(s); i += 2 {
        u = append(u, uint16(s[i])<<8|uint16(s[i+1]))
    }
    return string(utf16.Decode(u))
}

Could you simplify ucs2Encoder.Decode to return utf16Decode(raw)? That removes the encoding/binary and unicode/utf16 imports from page.go. (Note: utf16Decode panics on odd-length input while your version truncates; content codes are always 2-byte aligned so it should be fine, but a length guard is welcome if you want to be defensive.)

2. Make the test test only the encoder

Decode receives the already-unescaped string — the lexer readLiteralString handles \( / \nnn. The bytes.ReplaceAll(data, []byte{0x5C}, []byte{}) conflates that with the encoder and would silently corrupt any legitimate code containing a 0x5C byte. Could you pass the decoded UCS-2 bytes directly (e.g. 0x00, 0x28 for ( and 0x00, 0x29 for )) and drop the bytes import? If you want to document the raw on-disk form, a comment is better than manipulating the data in the test.

3. Minor: copyright header

The new page_test.go uses Copyright 2014 The Go Authors, but the repo's other test files (read_test.go, lex_test.go, ps_test.go) have no such header, and this code isn't from the Go team. Mind removing it for consistency?

Everything else looks good — thanks again!

@kvii

kvii commented Sep 5, 2026

Copy link
Copy Markdown
Author

@ledongthuc You're right. I changed my code.

@ledongthuc

Copy link
Copy Markdown
Owner

@kvii Hi, it's good to merge now. Just got a conflict from page_test.go file
Can you solve it, or I can help to solve if you not sure.

@ledongthuc

Copy link
Copy Markdown
Owner

@kvii seems the fix is failed. Could you check again?

@ledongthuc

Copy link
Copy Markdown
Owner

looks good!

@ledongthuc
ledongthuc merged commit 6c8c28e into ledongthuc:master Sep 7, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GetPlainText do not support encoding "UniGB-UCS2-H"

2 participants