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

feat: add UTF-8 with BOM support for CSV exported files #90

Merged
merged 2 commits into from
Feb 27, 2024

Conversation

flang
Copy link
Contributor

@flang flang commented Jan 23, 2024

Close #89

Copy link
Member

@javier-godoy javier-godoy left a comment

Choose a reason for hiding this comment

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

Should we use a SerializableSupplier<Charset> so that GridExporter remains serializable?

Copy link
Member

@javier-godoy javier-godoy left a comment

Choose a reason for hiding this comment

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

Note that charset is a captured argument, which is included in the serialized form of the supplier.

Exception in thread "main" org.apache.commons.lang3.SerializationException: java.io.NotSerializableException: sun.nio.cs.ISO_8859_1
	- element of array (index: 0)
	- array (class "[Ljava.lang.Object;", size: 1)
	- field (class "java.lang.invoke.SerializedLambda", name: "capturedArgs", type: "class [Ljava.lang.Object;")
	- object (class "java.lang.invoke.SerializedLambda", SerializedLambda
public class Test implements Serializable {

  SerializableSupplier<Charset> charsetSupplier;

  public Test(Charset charset) {
    charsetSupplier = () -> charset;
  }

  public Test(SerializableSupplier<Charset> charsetSupplier) {
    this.charsetSupplier = charsetSupplier;
  }

  public static void main(String[] args) {
    // SerializationUtils.clone(new Test(StandardCharsets.ISO_8859_1)); fails
    SerializationUtils.clone(new Test(() -> StandardCharsets.ISO_8859_1)); //works
  }

}

Since this change affects serialization in a non-trivial way, the feature should be configured in https://github.com/FlowingCode/GridExporterAddon/blob/master/src/test/java/com/flowingcode/vaadin/addons/gridexporter/test/SerializationTest.java#L49

Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@javier-godoy javier-godoy merged commit 5e78b24 into master Feb 27, 2024
3 checks passed
@javier-godoy javier-godoy deleted the issue-89 branch February 27, 2024 19:34
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.

How to use utf-8 charset for csv?
2 participants