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

Solve the problem of garbled characters #271

Merged
merged 4 commits into from
Jan 5, 2023
Merged

Conversation

lhDream
Copy link
Contributor

@lhDream lhDream commented Nov 26, 2022

Solve the problem that CJK characters cannot be packaged and installed packages and garbled characters. #202

@lhDream
Copy link
Contributor Author

lhDream commented Nov 26, 2022

https://stackoverflow.com/questions/51066207/utf-8-characters-not-displaying-correctly-in-inno-setup
The UTF-8 string output by java will not carry BOM

@fvarrui fvarrui changed the base branch from master to devel December 7, 2022 13:39
@fvarrui fvarrui added the feedback Waiting for feedback label Dec 7, 2022
@@ -47,7 +45,7 @@ public static ExecutionResult executeWithResult(File workingDirectory, String ex

Process process = command.execute();

BufferedReader output = new BufferedReader(new InputStreamReader(process.getInputStream()));
BufferedReader output = new BufferedReader(new InputStreamReader(process.getInputStream(), Charset.forName("GBK")));
Copy link
Owner

Choose a reason for hiding this comment

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

What is GBK for?

Copy link
Owner

Choose a reason for hiding this comment

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

Are you sure this won't present other problems with other language encodings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure.
maybe so:

BufferedReader output = new BufferedReader(new InputStreamReader(process.getInputStream(), CharSetUtil.getCommandLineChartSet()));
public class CharSetUtil {

    public static Charset getCommandLineChartSet(){
        try{
            Process p = Runtime.getRuntime().exec("cmd /k chcp");
            BufferedReader br = new BufferedReader(new InputStreamReader(p.getInputStream()));
            String res = br.readLine();
            String code = find("\\d+",res);
            switch (code){
                case "936": return Charset.forName("GBK");
                case "65001": return Charset.forName("UTF-8");
            }
        }catch (Exception e){
        }
        return return Charset.defaultCharset();
    }

    private static String find(String pattern,String data){
        Pattern r = Pattern.compile(pattern);
        Matcher matcher = r.matcher(data);
        matcher.find();
        return matcher.group();
    }

}

https://learn.microsoft.com/en-us/windows/win32/intl/code-page-bitfields

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @fvarrui
Do you have any questions?

@lhDream lhDream requested a review from fvarrui December 23, 2022 07:03
@fvarrui fvarrui changed the base branch from devel to pr-271 January 5, 2023 10:50
@fvarrui fvarrui merged commit 2d25ac8 into fvarrui:pr-271 Jan 5, 2023
@fvarrui
Copy link
Owner

fvarrui commented Jan 5, 2023

Hi @lhDream!
Thanks for your PR ... I'll give it a try asap.

@fvarrui
Copy link
Owner

fvarrui commented Jan 9, 2023

Please, take a look at this:

public static void render(String templatePath, File output, Object info) throws Exception {
render(templatePath, output, info, false);
}
public static void render(String templatePath, File output, Object info, boolean includeBom) throws Exception {
String data = render(templatePath, info);
if (!includeBom) {
writeStringToFile(output, StringUtils.dosToUnix(data), "UTF-8");
} else {
FileUtils.writeStringToFileWithBOM(output, data);
}
}

I reverted your changes in this part of the code, but keeping the possibility to render files with Velocity including the BOM ... We should only include the BOM in those files where it is essential, not in all files by default. Do you know which files really need it?

@lhDream
Copy link
Contributor Author

lhDream commented Jan 9, 2023

Hi @fvarrui

It is currently known that the iss.vtl file requires

@fvarrui
Copy link
Owner

fvarrui commented Jan 9, 2023

Ok, done!

// generates iss file from velocity template
File issFile = new File(assetsFolder, name + ".iss");
VelocityUtils.render("windows/iss.vtl", issFile, packager, true);

public static void render(String templatePath, File output, Object info, boolean includeBom) throws Exception {
String data = render(templatePath, info);
data = StringUtils.dosToUnix(data);
if (!includeBom) {
writeStringToFile(output, data, "UTF-8");
} else {
FileUtils.writeStringToFileWithBOM(output, data);
}
}

Could you test branch pr-272?
Thanks!

@lhDream
Copy link
Contributor Author

lhDream commented Jan 10, 2023

Hi @fvarrui
Of course, but there is no such branch as pr-272,or what you want to say is pr-271

I tested with pr-271 branch no problem.

@fvarrui
Copy link
Owner

fvarrui commented Jan 10, 2023

Yes, sorry, I mean 271 🙄

@lhDream
Copy link
Contributor Author

lhDream commented Jan 10, 2023

@fvarrui
I tested with pr-271 branch no problem.

@fvarrui
Copy link
Owner

fvarrui commented Jan 12, 2023

Merged into devel, ready to be released in the next version

@commi
Copy link
Contributor

commi commented Feb 13, 2023

FYI: This change breaks with the non-unicode InnoSetup 5.x version... it cant read the iss file.

This IS version is old, but this could be added to the documentation. (use 6.x or 5.x Unicode)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feedback Waiting for feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants