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

Linked values, line breaks issue and naming fixes #63

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<groupId>com.github.miachm.sods</groupId>
<artifactId>SODS</artifactId>
<packaging>jar</packaging>
<version>1.5.2</version>
<version>1.5.3</version>
Copy link
Owner

Choose a reason for hiding this comment

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

We use the next format in SODS for versioning:

X.Y.Z

X refers to API versions. Changes within the same X are compatible. The API of the version 1.1.0 is completely compatible with 1.5.0 (although not the other way around). The day we redesign the API, we need to update the X.
Y refers to additional features. There are aditional features in the API, although the API is still backwards-compatible.
Z refers to a bugfixing release. There are no aditional features or changes in the API

This is a new feature. Therefore, we need to update the Y version.


<name>Simple ODS library</name>
<description>A library for load/save ODS files in java.</description>
Expand Down Expand Up @@ -71,6 +71,7 @@
Bundle-SymbolicName: com.github.miachm.sods
Automatic-Module-Name: com.github.miachm.sods
-jpms-module-info: com.github.miachm.sods
-sources: true
]]></bnd>
</configuration>
<extensions>true</extensions>
Expand Down
28 changes: 25 additions & 3 deletions src/com/github/miachm/sods/Cell.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.github.miachm.sods;

import java.time.LocalDate;
import java.util.Objects;
import java.util.*;
Copy link
Owner

Choose a reason for hiding this comment

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

Why use an asterisk here?


class Cell extends TableField {
private Object value;
Expand All @@ -10,8 +10,10 @@ class Cell extends TableField {
private GroupCell group;
private OfficeAnnotation annotation;

Cell()
{
private List<LinkedValue> linkedValues;

Cell() {
linkedValues = new ArrayList<>();
}

GroupCell getGroup() {
Expand Down Expand Up @@ -56,6 +58,7 @@ void clear()
formula = null;
style = Style.default_style;
annotation = null;
linkedValues = new ArrayList<>();
}

String getFormula() {
Expand Down Expand Up @@ -102,6 +105,22 @@ public void setAnnotation(OfficeAnnotation annotation) {
this.annotation = annotation;
}

List<LinkedValue> getLinkedValues() {
return linkedValues;
}

boolean hasLinkedValues() {
return !linkedValues.isEmpty();
}

void setLinkedValues(List<LinkedValue> linkedValues) {
this.linkedValues = linkedValues;
}

void addLinkedValue(LinkedValue linkedValue) {
this.linkedValues.add(linkedValue);
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
Expand All @@ -124,6 +143,7 @@ public boolean equals(Object o) {
if (!Objects.equals(formula, cell.formula)) return false;
if (!Objects.equals(annotation, cell.annotation)) return false;
if (!Objects.equals(num_repeated, cell.num_repeated)) return false;
if (!Objects.equals(linkedValues, cell.linkedValues)) return false;
return style.equals(cell.getStyle());
}

Expand All @@ -139,6 +159,7 @@ public int hashCode()
result = 31 * result + style.hashCode();
result = 31 * result + (group != null ? group.hashCode() : 0);
result = 31 * result + annotation.hashCode();
result = 31 * result + linkedValues.hashCode();
return result;
}

Expand All @@ -147,6 +168,7 @@ public String toString() {
if (getGroup() == null || getGroup().getCell() == this)
return "Cell{" +
"value=" + value +
", linkedValues='" + Arrays.toString(linkedValues.toArray()) + '\'' +
", formula='" + formula + '\'' +
", style=" + style +
", num_repeated=" + num_repeated +
Expand Down
87 changes: 87 additions & 0 deletions src/com/github/miachm/sods/LinkedValue.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
package com.github.miachm.sods;

import java.io.File;
import java.net.URI;
import java.net.URL;
import java.util.Objects;

/**
* Represents a value linked to a sheet, file, URI or URL.
*/
public class LinkedValue {
private final String href;
private final String value;
Copy link
Owner

Choose a reason for hiding this comment

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

Why a value? I thought this feature was for linking a cell to another.

Choose a reason for hiding this comment

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

Hi Miguel,

To address your comments:

  • You must have missed the fact that version was bumped with first commit, where bug fix was made; I am quite familiar with semantic versioning (https://semver.org/), I work with OSGi on a daily basis; you are free to bump it to whatever version you'd like; I needed to build the jar and include in local BND repo so I can continue on a different project where SODS library is consumed;
  • As it is mentioned in Javadoc of LinkedValue class, it is used to link a value to a sheet, file, URI or URL; this is a feature of ODS spec, perhaps you should check the generated XML code and/or generate an ODS file and open it with e.g. LibreOffice and you'll see how clicking a cell value with take you to another sheet, etc.;
  • I only need write capability, therefore I only focused on that; I do not really have the time to contribute more to this project at this point - that is all that is needed in a different project which consumes SODS library; if you or anyone else feels the need to add read capability, you are free to do so;
  • Separate class as well as builder pattern in LinkedValue was done on purpose, with hope that with time abstractions for other elements can also be created, a la https://github.com/jferard/fastods/, which is designed really well, unfortunately uses prohibitive license; currently, in SODS, everything is crammed together in ODSWriter, the elements do really know their XML representation, etc.;
  • A1 is for compatibility with Excel, where sheet links written as #SheetName do not work in Excel, but #SheetName.A1 work in both Excel and LibreOffice;
  • I am not really sure how to answer the asterisk question - I worked on this using IDEA IDE, and this is how it optimizes imports;
  • I did not have a timer on, but somewhere around 4+ hours, including time needed to analyze and come up with solution, plus additional integration test cases;

Feel free to use this as you'd like, I know I am already using this in a project where SODS is consumed. Sorry I do not have more time to help now.

Copy link
Owner

Choose a reason for hiding this comment

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

Don't get me wrong, I am grateful for any contribution. My comments try to balance between being appreciative and giving meaningful feedback according to the library standards. But sometimes I can excesively direct. I understand not everyone have the time to fully contribute to the development (dam, I don't even have the time myself).

Yep, some classes in SODS are becoming in god classes, specially ODSReader/ODSWritter like you mentioned. Nevertheless I try to keep the external-user API as consistent as posible, that's why the build pattern does not fit with the current implementation, because it presents to the user a completely different style. Eventually I'll need to deprecate the API and re-design it, but, you know, not enough time!

Thanks again, let's see if I can finish the feature and release another version.


LinkedValue(String href, String value) {
this.href = href;
this.value = value;
}

public static LinkedValue.Builder builder() {
Copy link
Owner

Choose a reason for hiding this comment

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

This is the builder pattern. Althought I understand the value of the pattern, is not consistent with the rest of API. SODS usually uses a more direct-straightforward style to the user.

return new LinkedValue.Builder();
}

String getHref() {
return href;
}

String getValue() {
return value;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
LinkedValue that = (LinkedValue) o;
return Objects.equals(href, that.href) && Objects.equals(value, that.value);
}

@Override
public int hashCode() {
return Objects.hash(href, value);
}

@Override
public String toString() {
return "LinkedValue{" +
"href='" + href + '\'' +
", value=" + value +
'}';
}

public static class Builder {
private String href;
private String value;

public LinkedValue build() {
return new LinkedValue(href, value);
}

public LinkedValue.Builder value(String value) {
this.value = value;
return this;
}

public LinkedValue.Builder href(Sheet sheet) {
this.href = '#' + sheet.getName() + ".A1";
Copy link
Owner

Choose a reason for hiding this comment

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

Why A1?

// this.href = '#' + sheet.getName(); // TODO: remove this - not compatible with Excel
return this;
}

public LinkedValue.Builder href(File file) {
this.href = file.toURI().toString();
return this;
}

public LinkedValue.Builder href(URL url) {
this.href = url.toString();
return this;
}

public LinkedValue.Builder href(URI uri) {
this.href = uri.toString();
return this;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,21 @@
import javax.xml.stream.XMLStreamWriter;
import java.io.*;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

/**
* Internal class for generate ODS files.
*/
class OdsWritter {
class OdsWriter {
private final static String office = "urn:oasis:names:tc:opendocument:xmlns:office:1.0";
private final static String table_namespace = "urn:oasis:names:tc:opendocument:xmlns:table:1.0";
private final static String text_namespace = "urn:oasis:names:tc:opendocument:xmlns:text:1.0";
private final static String font_namespace = "urn:oasis:names:tc:opendocument:xmlns:xsl-fo-compatible:1.0";
private final static String style_namespace = "urn:oasis:names:tc:opendocument:xmlns:style:1.0";
private final static String metadata_namespace = "http://purl.org/dc/elements/1.1/";
private final static String datatype_namespace ="urn:oasis:names:tc:opendocument:xmlns:datastyle:1.0";
private final static String xlink_namespace ="http://www.w3.org/1999/xlink";

private SpreadSheet spread;
private Compressor out;
Expand All @@ -27,14 +29,14 @@ class OdsWritter {
private Map<TableStyle, String> tableStyleStringMap = new HashMap<>();
private final String MIMETYPE= "application/vnd.oasis.opendocument.spreadsheet";

private OdsWritter(OutputStream o, SpreadSheet spread) {
private OdsWriter(OutputStream o, SpreadSheet spread) {
this.spread = spread;
this.out = new Compressor(o);
spread.trimSheets();
}

public static void save(OutputStream out,SpreadSheet spread) throws IOException {
new OdsWritter(out,spread).save();
new OdsWriter(out,spread).save();
}

private void save() throws IOException {
Expand Down Expand Up @@ -108,6 +110,7 @@ private void writeSpreadsheet() throws UnsupportedEncodingException, XMLStreamEx
out.writeAttribute("xmlns:style", style_namespace);
out.writeAttribute("xmlns:dc", metadata_namespace);
out.writeAttribute("xmlns:number", datatype_namespace);
out.writeAttribute("xmlns:xlink", xlink_namespace);

out.writeAttribute("office:version", "1.2");

Expand Down Expand Up @@ -263,39 +266,61 @@ private void setCellStyle(XMLStreamWriter out, Style style) throws XMLStreamExce
}

private void writeValue(XMLStreamWriter out, Cell cell) throws XMLStreamException {
Object v = cell.getValue();
if (v != null) {
OfficeValueType valueType = OfficeValueType.ofJavaType(v.getClass());
valueType.write(v, out);

out.writeStartElement("text:p");
String text = v.toString();

for (int i = 0; i < text.length(); i++) {
if (text.charAt(i) == ' ') {
out.writeStartElement("text:s");
int cnt = 0;
while (i+cnt < text.length() && text.charAt(i + cnt) == ' ') {
cnt++;
if (!cell.hasLinkedValues()) {
Object v = cell.getValue();
if (v != null) {
OfficeValueType valueType = OfficeValueType.ofJavaType(v.getClass());

String text = v.toString();
if (text.contains(System.lineSeparator())) {
valueType.write("", out);

out.writeStartElement("text:p");

for (int i = 0; i < text.length(); i++) {
if (text.charAt(i) == ' ') {
out.writeStartElement("text:s");
int cnt = 0;
while (i + cnt < text.length() && text.charAt(i + cnt) == ' ') {
cnt++;
}
if (cnt > 1)
out.writeAttribute("text:c", "" + cnt);
i += cnt - 1;
out.writeEndElement();
} else if (text.charAt(i) == '\t') {
out.writeEmptyElement("text:tab");
} else if (text.charAt(i) == '\n') {
out.writeEndElement();
out.writeStartElement("text:p");
} else
out.writeCharacters("" + text.charAt(i));
}
if (cnt > 1)
out.writeAttribute("text:c", "" + cnt);
i += cnt - 1 ;

out.writeEndElement();

} else {
valueType.write(v, out);
}
else if (text.charAt(i) == '\t') {
out.writeEmptyElement("text:tab");
}
else if (text.charAt(i) == '\n') {
out.writeEndElement();
}
} else {
List<LinkedValue> links = cell.getLinkedValues();
if (links != null && !links.isEmpty()) {
OfficeValueType valueType = OfficeValueType.ofJavaType(String.class);
valueType.write("", out);

for (LinkedValue link : links) {
out.writeStartElement("text:p");
out.writeStartElement("text:a");
out.writeAttribute("xlink:href", link.getHref());
out.writeAttribute("xlink:type", "simple");
out.writeCharacters(link.getValue());
out.writeEndElement();
out.writeEndElement();
}
else
out.writeCharacters("" + text.charAt(i));
}

out.writeEndElement();
}

OfficeAnnotation annotation = cell.getAnnotation();
if (annotation != null) {
out.writeStartElement("office:annotation");
Expand Down
Loading