Can you accept my pull request?

Hello, I am using your library in my project https://github.com/wilinz/i18n-translator, first of all thank you very much,
I found some problems during use, as follows,

In the store method, write did not call flush() in time, resulting in no real-time output.
The hex length of unicode escape may not have 4 bits, resulting in wrong output, such as Arabic.
From JDK9 and above, java already supports utf8 properties file, so I add several store methods, which add the option of encoding unicode or not.

I have fixed and tested these problems, but I changed too much and used the gradle build tool instead of maven, so I want to ask if you can accept my pull request,
This library does not use kotlin, but build.gradle.kts uses kotlin, so github will show that 1% of kotlin is used.

resulting in no real-time output.

Can you explain what you mean by this? Because I don't really understand the necessity for all those flush() calls, they should be unnecessary and might even hurt performance in some cases. Can you tell me what the problem was you're trying to solve?

The hex length of unicode escape may not have 4 bits, resulting in wrong output, such as Arabic.

Can you give an example of this? (A test case would be nice)

From JDK9 and above, java already supports utf8 properties file, so I add several store methods, which add the option of encoding unicode or not.

Unfortunately the library needs to be compatible with JDK 8, so we can't use features from 9. Hopefully we can upgrade to a newer JDK in the future but not right now.

From JDK9 and above, java already supports utf8 properties file, so I add several store methods, which add the option of encoding unicode or not.

Unfortunately the library needs to be compatible with JDK 8, so we can't use features from 9. Hopefully we can upgrade to a newer JDK in the future but not right now.

I made it backward compatible, unicode encoding is optional, if you want to be compatible with JDK8, isEncodeUnicode just need to pass in true, or use the original store method (isEncodeUnicode defaults to true).

From JDK9 and above, java already supports utf8 properties file, so I add several store methods, which add the option of encoding unicode or not.

Unfortunately the library needs to be compatible with JDK 8, so we can't use features from 9. Hopefully we can upgrade to a newer JDK in the future but not right now.

    @Test
    void testStore() throws IOException, URISyntaxException {
        Properties p = new Properties()
        p.setProperty("test","الألبانية");
        StringWriter sw = new StringWriter();
        p.store(sw);
        System.out.println(sw);
    }

Expected output: test=\u0627\u0644\u0623\u0644\u0628\u0627\u0646\u064a\u0629
Actual output: test=\u627\u644\u623\u644\u628\u627\u646\u64a\u629

resulting in no real-time output.

Can you explain what you mean by this? Because I don't really understand the necessity for all those flush() calls, they should be unnecessary and might even hurt performance in some cases. Can you tell me what the problem was you're trying to solve?

flush() will immediately output the string in the buffer to the stream, otherwise you won't see the output until the buffer is full, without calling flush(), you won't see any output, for example:

@Test
    void testStore() throws IOException, URISyntaxException {
        Properties p = new Properties()
        p.setProperty("test","test");
        p.store(System.out);
    }

You will not see any output, if you call flush() you will see the output

Fix codejive#8

You will not see any output, if you call flush() you will see the output

Ok, but I think it's a much better solution to leave that to the caller, like this:

Properties p = new Properties()
p.setProperty("test","test");
p.store(System.out);
System.out.flush();

Expected output: test=\u0627\u0644\u0623\u0644\u0628\u0627\u0646\u064a\u0629
Actual output: test=\u627\u644\u623\u644\u628\u627\u646\u64a\u629

Aha! I understand, that is indeed wrong.
I'll make a separate issue for this: #11

I also created an issue for the UTF8 handling: #13

@wilinz

Hi @wilinz , I've created a new PR #14 that does NO automatic escaping of characters anymore except for the ones required by the .properties file format (eg. \n, \r, \f, etc).

This means that files are left intact, if you have a file like:

item1=الألبانية
item2=\u0627\u0644\u0623\u0644\u0628\u0627\u0646\u064a\u0629

and you load() it and you store() it again it will stay exactly the same (which is exactly how it should have always worked to be honest).

But in case that you want to specifically write to a ISO8859-1 file you can first call props.escaped() which will return a new Properties object with all Unicode characters above 0xFF turned into escape sequences. (There's also an props.unescaped() which does the opposite).

Will this fix your issues?

Thanks, but I forked and made some customizations, my plugin has been developed, you can try it if you are interested
https://github.com/wilinz/globalization-translator

Ok, anyway, thanks for your suggestions, they we very useful for rethinking some of the Unicode handling.
And your plugin looks interesting, I'm sure to take a look if I need to do some i18n.

Navigation Menu

Repository navigation

Can you accept my pull request? #8

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Can you accept my pull request? #8

Description

Activity

quintesse commented on Jul 27, 2022

quintesse commented on Jul 27, 2022

quintesse commented on Jul 27, 2022

wilinz commented on Jul 28, 2022

wilinz commented on Jul 28, 2022

wilinz commented on Jul 28, 2022

quintesse commented on Jul 28, 2022

quintesse commented on Jul 28, 2022

quintesse commented on Jul 28, 2022

quintesse commented on Jul 29, 2022

wilinz commented on Jul 30, 2022

quintesse commented on Jul 30, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions