Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support -bom in encoding option and detect BOM #3396

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

niten94
Copy link
Contributor

@niten94 niten94 commented Jul 19, 2024

BOM is read and written in files if encoding option ends in -bom in this pull request. There are not much encodings supporting BOM so the encoding has to be checked when BOM is used. A new option where usage of BOM is enabled is not added because both options cannot be validated when one is set.

The option is set when BOM is detected using utfbom so utfbom is added as a dependency package. Setting encoding to UTF-32 is not supported so the option is not set when UTF-32 is detected. Bytes that are not valid UTF-8 text are modified so UTF-32 BOM is intentionally not included in buffer.

BOM is not written in empty files when transform.Writer.Close is not called in overwriteFile in internal/buffer/save.go so the method is called. It has been not tested if there is an unexpected bug with calling the method.

Closes #3369

@niten94 niten94 changed the title Support -bom in encoding option and detect BOM Support -bom in encoding option and detect BOM Jul 19, 2024
@niten94
Copy link
Contributor Author

niten94 commented Aug 29, 2024

The points below have to be improved but I do not know when I will have time, so I will convert this pull request to a draft:

  • naming convention and code style is different with other parts of code
  • behavior in 212299a is weird
  • pull request description is not easy to understand

@niten94 niten94 marked this pull request as ready for review September 7, 2024 14:30
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.

support BOM head on win , linux
1 participant