Skip to content

Detect file encoding from byte order mark - #235

Open
dronov-dmitry wants to merge 2 commits into
maxistar:masterfrom
dronov-dmitry:auto-encoding-detection
Open

Detect file encoding from byte order mark#235
dronov-dmitry wants to merge 2 commits into
maxistar:masterfrom
dronov-dmitry:auto-encoding-detection

Conversation

@dronov-dmitry

Copy link
Copy Markdown
Contributor

Summary

Fixes opening files that are not UTF-8: the editor used the configured default encoding (UTF-8) for every file, so e.g. a UTF-16LE file was shown as garbage. Now the file encoding is detected from its byte order mark (BOM) and the file is opened (and saved) in the correct encoding.

How it works

  • New utils/FileEncoding detects the BOM:
    • UTF-32 LE/BE, UTF-8, UTF-16 BE/LE (in order of signature length so the 4-byte UTF-32 LE signature is not misread as UTF-16 LE).
  • On open (openNamedFileDirect, openNamedFileLegacyDirect, applyExternalDocument) the detected encoding is used and the BOM is stripped from the text.
  • On save the document is written back in its original encoding with the BOM restored, so the file on disk stays unchanged in format.
  • When no BOM is present, the configured default encoding is used as before.

Verification

  • New unit tests FileEncodingTest (10 cases: detection, BOM stripping, encode round-trip) — all pass.
  • ./gradlew compileDebugJavaWithJavac — OK
  • ./gradlew testDebugUnitTest — OK
  • ./gradlew lint — no errors
  • Verified against a real UTF-16LE (BOM) Cyrillic report file.

@maxistar maxistar left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is a very valuable improvement and the BOM detection/round-trip implementation looks good. I found one data-integrity edge case: after restoring an edited UTF-16/UTF-32 recovery draft, documentEncoding is null, so the next save may rewrite the file using the configured default encoding and drop its BOM. The recovery metadata already has encoding and hasBom; could we persist both and reconstruct documentEncoding in restoreDraft()? A recovery test covering UTF-16LE+BOM → edit → recreate/restore → save would be ideal.

…raft()

- Add FileEncoding.fromCharset() to reconstruct encoding from metadata
- restoreDraft() now restores documentEncoding from draft metadata
- createRecoverySnapshot() uses documentEncoding.hasBom() instead of hardcoded false
- Fix RecoveryMetadata JSON null handling for encoding field
- Add UTF-16LE+BOM round-trip recovery test
@dronov-dmitry

Copy link
Copy Markdown
Contributor Author

Thanks for detailed description of the changes!

To ensure everything works reliably, it would be great to write unit tests covering all these edge cases you've listed. Also, please make sure to include tests for opening files in various character encodings to prevent any regressions there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants