Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[calligra] Fix issues with formatting of currency values. Fixes JB#47521
Fix conditional styles begin incorrectly identified as being equal and
grouped together.

Fix internal currency formatting and spreadsheet defined cell formatting
both being applied to a cell leading to duplicate currency symbols.

Fix the xlsx style format parsing skipping the number format information
if it follows the currency symbol leading to cells only showing the
symbol and no numbers.
  • Loading branch information
adenexter committed Oct 15, 2019
1 parent 690465b commit ff69d4a
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 7 deletions.
67 changes: 67 additions & 0 deletions rpm/calligra-conditional-styles.patch
@@ -0,0 +1,67 @@
From dd5370f0e37bb4a481d433cff915c9e282b0d218 Mon Sep 17 00:00:00 2001
From: Andrew den Exter <andrew.den.exter@qinetic.com.au>
Date: Mon, 14 Oct 2019 06:54:40 +0000
Subject: [PATCH 1/3] Fix incorrect conditional styling of spreadsheet cells.

Include the conditions default style when comparing equality and
calculating the hash to reduce instances of false equivalency.

Then remove the QMap index cache since sorting on a hash an item may
still produce unstable results in the somewhat unlikely case of a hash
collision and it's going to take a lot of items before building and
searching the map starts to be less expensive than a linear scan.
---
sheets/Condition.cpp | 4 +++-
sheets/RectStorage.h | 6 +-----
2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/sheets/Condition.cpp b/sheets/Condition.cpp
index f01f0e1..c1cf7f5 100644
--- a/sheets/Condition.cpp
+++ b/sheets/Condition.cpp
@@ -381,6 +381,8 @@ void Conditions::operator=(const Conditions & other)

bool Conditions::operator==(const Conditions& other) const
{
+ if (d->defaultStyle != other.d->defaultStyle)
+ return false;
if (d->conditionList.count() != other.d->conditionList.count())
return false;
QLinkedList<Conditional>::ConstIterator end(d->conditionList.end());
@@ -399,7 +401,7 @@ bool Conditions::operator==(const Conditions& other) const

uint Calligra::Sheets::qHash(const Conditions &c)
{
- uint res = 0;
+ uint res = qHash(c.defaultStyle());
foreach (const Conditional& co, c.conditionList()) {
res ^= qHash(co);
}
diff --git a/sheets/RectStorage.h b/sheets/RectStorage.h
index c9f77ab..c6f00a2 100644
--- a/sheets/RectStorage.h
+++ b/sheets/RectStorage.h
@@ -570,19 +570,15 @@ void RectStorageLoader<T>::run()

QList<QPair<QRegion, T> > treeData;
typedef QPair<QRegion, T> TRegion;
- QMap<T, int> indexCache;
foreach (const TRegion& tr, m_data) {
const QRegion& reg = tr.first;
const T& d = tr.second;

- typename QMap<T, int>::iterator idx = indexCache.find(d);
- int index = idx != indexCache.end() ? idx.value() : m_storage->m_storedData.indexOf(d);
+ int index = m_storage->m_storedData.indexOf(d);
if (index != -1) {
treeData.append(qMakePair(reg, m_storage->m_storedData[index]));
- if (idx == indexCache.end()) indexCache.insert(d, index);
} else {
treeData.append(tr);
- if (idx == indexCache.end()) indexCache.insert(d, m_storage->m_storedData.size());
m_storage->m_storedData.append(d);
}
}
--
1.8.3-rc3

30 changes: 30 additions & 0 deletions rpm/calligra-double-formatting.patch
@@ -0,0 +1,30 @@
From 6951e1e9b2d4439c83f385b354d246bd24e53ce3 Mon Sep 17 00:00:00 2001
From: Andrew den Exter <andrew.den.exter@qinetic.com.au>
Date: Mon, 14 Oct 2019 08:39:21 +0000
Subject: [PATCH 2/3] Don't double format spreadsheet currency values with
custom format strings.

---
sheets/ValueFormatter.cpp | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sheets/ValueFormatter.cpp b/sheets/ValueFormatter.cpp
index 66914b1..2f2d703 100644
--- a/sheets/ValueFormatter.cpp
+++ b/sheets/ValueFormatter.cpp
@@ -330,7 +330,11 @@ QString ValueFormatter::createNumberFormat(Number value, int precision,
break;
case Format::Money:
// There is no substitute for this in QLocale (toCurrencyString cannot set precision) :(
- localizedNumber = m_converter->settings()->locale()->formatMoney(val, currencySymbol.isEmpty() ? m_converter->settings()->locale()->currencySymbol() : currencySymbol, p);
+ if (_formatString.isEmpty()) {
+ localizedNumber = m_converter->settings()->locale()->formatMoney(val, currencySymbol, p);
+ } else {
+ localizedNumber = m_converter->settings()->locale()->formatNumber(val, p);
+ }
break;
case Format::Scientific: {
const QString decimalSymbol = m_converter->settings()->locale()->decimalSymbol();
--
1.8.3-rc3

27 changes: 27 additions & 0 deletions rpm/calligra-xlsx-styling.patch
@@ -0,0 +1,27 @@
From c70d858b620b6f9f21be639933ba923587765816 Mon Sep 17 00:00:00 2001
From: Andrew den Exter <andrew.den.exter@qinetic.com.au>
Date: Tue, 15 Oct 2019 06:31:51 +0000
Subject: [PATCH 3/3] Don't drop number styling information from xlsx cell
formats which start with a currency symbol.

---
filters/libmso/NumberFormatParser.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/filters/libmso/NumberFormatParser.cpp b/filters/libmso/NumberFormatParser.cpp
index b8d6734..21b2f80 100644
--- a/filters/libmso/NumberFormatParser.cpp
+++ b/filters/libmso/NumberFormatParser.cpp
@@ -260,7 +260,8 @@ KoGenStyle NumberFormatParser::parse(const QString& origNumberFormat, KoGenStyle
// do following only if we are really a number and not part of another KoGenStyle like a date or time formatting
if (type == KoGenStyle::NumericNumberStyle
|| type == KoGenStyle::NumericFractionStyle
- || type == KoGenStyle::NumericScientificStyle)
+ || type == KoGenStyle::NumericScientificStyle
+ || type == KoGenStyle::NumericCurrencyStyle)
{
bool grouping = false;
bool gotDot = false;
--
1.8.3-rc3

20 changes: 13 additions & 7 deletions rpm/calligra.spec
Expand Up @@ -31,14 +31,17 @@ Patch12: calligra-libs.patch
Patch13: calligra-plugins.patch
Patch14: calligra-error-reporting.patch
Patch15: calligra-sheets.patch
Patch16: calligra-conditional-styles.patch
Patch17: calligra-double-formatting.patch
Patch18: calligra-xlsx-styling.patch
# to be removed after Qt upgrade
Patch16: calligra-sheets-read-time.patch
Patch18: calligra-cache.patch
Patch19: calligra-qtdbus.patch
Patch20: calligra-background.patch
Patch21: calligra-rtf.patch
Patch22: calligra-invalidate-cache.patch
Patch23: calligra-gcc6.patch
Patch19: calligra-sheets-read-time.patch
Patch20: calligra-cache.patch
Patch21: calligra-qtdbus.patch
Patch22: calligra-background.patch
Patch23: calligra-rtf.patch
Patch24: calligra-invalidate-cache.patch
Patch25: calligra-gcc6.patch

%description
%{summary}.
Expand Down Expand Up @@ -188,12 +191,15 @@ BuildRequires: extra-cmake-modules >= 5.34.0
%patch14 -d upstream -p1
%patch15 -d upstream -p1
%patch16 -d upstream -p1
%patch17 -d upstream -p1
%patch18 -d upstream -p1
%patch19 -d upstream -p1
%patch20 -d upstream -p1
%patch21 -d upstream -p1
%patch22 -d upstream -p1
%patch23 -d upstream -p1
%patch24 -d upstream -p1
%patch25 -d upstream -p1

%define build_kf5() cd %1 ; if [ ! -d build ] ; then mkdir build ; fi ; cd build ; if [ ! -e Makefile ] ; then CMAKE_PREFIX_PATH=%{_buildrootdir}/kf5/usr cmake -DCMAKE_INSTALL_PREFIX=/usr -DCMAKE_INSTALL_RPATH=/usr/lib/calligra-kf5 %{?2} .. ; fi ; make %{?_smp_mflags} install DESTDIR=%{_buildrootdir}/kf5 ; cd ../.. ;
%build
Expand Down

0 comments on commit ff69d4a

Please sign in to comment.