Linked values, line breaks issue and naming fixes #63

ideas-into-software · 2023-03-26T19:29:06Z

Added support for linked values, i.e. linking cell value to a sheet, file, URI or URL
Fixed bug with cell values which contain line separators - setting those via both office:string-value attribute and text:p element results in office:string-value taking precedence and text displayed containing no line breaks
Added BND instruction to include sources in OSGi jar
Fixed spelling mistake in OdsWriter naming
Bumped version to 1.5.3

… those via both 'office:string-value' attribute and 'text:p' element results in 'office:string-value' taking precedence and text displayed containing no line breaks - Added BND instruction to include sources in OSGi jar - Fixed spelling mistake in 'OdsWriter' naming - Bumped version to 1.5.3

…, file, URI or URL

miachm · 2023-03-26T22:35:12Z

Thanks!

I am confused with the 2 PR... So is this the right one?

ideas-into-software · 2023-03-27T10:56:51Z

Hi Miguel,

Yes, this (#63) is the PR I would ask you review and merge.

The other PR (#62) was closed in favor of this PR, which contains both (bug fixes as well as support for linked values).

Thanks!

I am confused with the 2 PR... So is this the right one?

miachm

Thanks for the work, it seems you spent a considerable amount to implement this feature.

I still not sure how this feature works though. Is some kind of hyperlink to another cell? Did I understand right?

Also the feature is not finished. I miss the part of ODSReader for reading a file with this feature.

miachm · 2023-03-27T17:35:59Z

pom.xml

@@ -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>


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.

miachm · 2023-03-27T17:36:24Z

src/com/github/miachm/sods/Cell.java

@@ -1,7 +1,7 @@
 package com.github.miachm.sods;

 import java.time.LocalDate;
-import java.util.Objects;
+import java.util.*;


Why use an asterisk here?

miachm · 2023-03-27T17:50:46Z

src/com/github/miachm/sods/LinkedValue.java

+        this.value = value;
+    }
+
+    public static LinkedValue.Builder builder() {


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.

miachm · 2023-03-27T17:51:24Z

src/com/github/miachm/sods/LinkedValue.java

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


miachm · 2023-03-27T17:57:12Z

src/com/github/miachm/sods/LinkedValue.java

+ */
+public class LinkedValue {
+    private final String href;
+    private final String value;


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

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.

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.

juergen-albert · 2024-01-16T12:43:26Z

@miachm does anything speaks against merging the current state of this? We'd really like to use this and it would be great if we could just use a Version from maven Central instead of one from our own fork.

miachm · 2024-01-17T23:47:16Z

@juergen-albert I think I just forgot about it. Now it need to fix the conflicts t.t, but yes, it can be added to the next version.

juergen-albert · 2024-01-19T10:42:21Z

This is highly appreciated. If you need a hand, just reach out.

ideas-into-software added 2 commits March 24, 2023 21:51

- Added support for linked values, i.e. linking cell value to a sheet…

ffccb3f

…, file, URI or URL

ideas-into-software mentioned this pull request Mar 26, 2023

Line breaks issue and naming fixes #62

Closed

miachm reviewed Mar 27, 2023

View reviewed changes

ideas-into-software mentioned this pull request Mar 28, 2023

Replaced FastODS with SODS geckoprojects-org/org.gecko.emf.utils#8

Merged

LoadingByte mentioned this pull request May 22, 2023

Support writing strings with line breaks #65

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Linked values, line breaks issue and naming fixes #63

Linked values, line breaks issue and naming fixes #63

Linked values, line breaks issue and naming fixes #63

Are you sure you want to change the base?

Linked values, line breaks issue and naming fixes #63

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment