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

Supports UTF-8 file with BOM. #1693

Merged
merged 8 commits into from
May 21, 2022

Conversation

tifish
Copy link
Contributor

@tifish tifish commented Apr 30, 2022

The code may be a bit rough:

  • Does it break any convension?
  • Does it cover all cases?
  • The icon may be inappropriate. I just pick one from Android Studio.

@gsantner
Copy link
Owner

As of icon, that is OK - thats just what everybody else did so far here :D

Copy link
Owner

@gsantner gsantner left a comment

Choose a reason for hiding this comment

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

^

@gsantner gsantner added this to the Markor v2.10 milestone May 14, 2022
@gsantner gsantner force-pushed the master branch 4 times, most recently from 8bbdc13 to b9c45c3 Compare May 15, 2022 02:17
@tifish
Copy link
Contributor Author

tifish commented May 16, 2022

I committed all changes. See if I missed something.

@gsantner
Copy link
Owner

OK should I try it on the weekend?

@tifish
Copy link
Contributor Author

tifish commented May 19, 2022

Yeah, tell me if anything I can do.

Comment on lines +180 to 186
if (appSettings.getNewFileDialogLastUsedUtf8Bom()) {
arg_fos.write(0xEF);
arg_fos.write(0xBB);
arg_fos.write(0xBF);
}
if (templateContents != null && (!f.exists() || f.length() < ShareUtil.MIN_OVERWRITE_LENGTH)) {
arg_fos.write(templateContents);
Copy link
Owner

@gsantner gsantner May 21, 2022

Choose a reason for hiding this comment

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

We should get rid of own file writing implementation here and instead use FileUtils, what is used at editor as well.

But that is a topic of it's own.

@gsantner gsantner merged commit 2b54a76 into gsantner:master May 21, 2022
@harshad1
Copy link
Collaborator

harshad1 commented May 25, 2022

@tifish FYI, The bom test in Document #189 is causing a crash for me on start. The bug does not appear in debug mode, just release mode.

Also, why are we returning a map from readTextFileFast insead of a simple boolean?

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.

3 participants