Re: [PATCH] fdo#50950 modify calc functions WEEKNUM(_ADD) to comply with ODFF1.2
Hi Winfried, On Tuesday, 2012-11-20 13:24:06 +0100, Winfried Donkers wrote: The link to the help files has been broken, i.e. WEEKNUM now points to the (partially out of date) help text for WEEKNUM_ADD, but ISOWEEKNUM does not point to a help text. That link wasn't broken, the xhp files were not included in the patch. I guess it is beyond the scope for git and that I should use ./g instead to include it. Am I correct here? Hm.. the dreaded submodules.. ;) Please see http://wiki.documentfoundation.org/Development/Submodules and especially http://wiki.documentfoundation.org/Development/Submodules#What_is_the_impact_for_developers_working_in_the_auxiliary_repositories_.3F Eike -- LibreOffice Calc developer. Number formatter stricken i18n transpositionizer. GnuPG key 0x293C05FD : 997A 4C60 CE41 0149 0DB3 9E96 2F1A D073 293C 05FD Support the FSFE, care about Free Software! https://fsfe.org/support/?erack pgpYtMi072Lso.pgp Description: PGP signature ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
RE: [PATCH] fdo#50950 modify calc functions WEEKNUM(_ADD) to comply with ODFF1.2
Hi Eike, One important matter is still open: When opening calc-documents that use the 'old' functions, they are not always computed correctly. The 'new' functions differ in paramter count. I do not know where to add code for backward compatibility when opening these documents, but with some help/hints I might be able to fix that. There are three cases to be considered: 1. reading ODF 1.2 (ODFF) 2. writing ODF 1.0/1.1 (PODF) 3. reading ODF 1.0/1.1 (PODF) [...] I'm afraid I'm lost here. I can't find my way in/around FormulaCompiler::Factor(). To put it simple: with respect to calc-formulas I understand what the user sees and I (often) understand what's happening in ScInterpreter. But the bit inbetween as rather foggy for me. And to be honest, I doubt if a thorough study of that area will help me much. So I've got another three cases to consider :-/ 1. Give up fdo50950 - I don't like this option, not vanity but I would the functions itself to be available in LibO 3. Get some help from co-developers who are familiar with this kind of poking - that would be nice, but is out of my control 2. Struggle on in the hope of magically getting there - this will be the default as I don't like case 1 I will see that the submodules are included in the patch sofar that I attached to the bug itself and concentrate on other bugs (like IFERROR/IFNA/NUMBERVALUE). Winfried ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
RE: [PATCH] fdo#50950 modify calc functions WEEKNUM(_ADD) to comply with ODFF1.2
Hi Eike, I'm currently putting your review into a better patch: -renaming the functions: I reverted some lines as you proposed. The link to the help files has been broken, i.e. WEEKNUM now points to the (partially out of date) help text for WEEKNUM_ADD, but ISOWEEKNUM does not point to a help text. That link wasn't broken, the xhp files were not included in the patch. I guess it is beyond the scope for git and that I should use ./g instead to include it. Am I correct here? There are three cases to be considered: 1. reading ODF 1.2 (ODFF) 2. writing ODF 1.0/1.1 (PODF) 3. reading ODF 1.0/1.1 (PODF) This will need some study from me, poking is not my strongest capabilty. I will put the latest diff file as an attachment in bug 50950, as I'm afraid the 'poking' will take some time. Winfried ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
RE: [PATCH] fdo#50950 modify calc functions WEEKNUM(_ADD) to comply with ODFF1.2
Hi Winfried, a common question in ML, Ask and forums, is about the functions: SUMIFS 6.16.63, AVERAGEIFS 6.18.6, COUNTIFS 6.13.10 (OpenDocument-v1.2-part2) People often do not understand very well how to use SUMPRODUCT() or matrix formulas, and with them, is more complicate the use of regular expressions. I do not use MS in anyway but I think this functions are there, and if I not in a mistake, also will be introduced in AOO. I think they could be very helpful, and good for a best interoperability. Miguel Ángel. -- View this message in context: http://nabble.documentfoundation.org/PATCH-fdo-50950-modify-calc-functions-WEEKNUM-ADD-to-comply-with-ODFF1-2-tp4011230p4018927.html Sent from the Dev mailing list archive at Nabble.com. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
RE: [PATCH] fdo#50950 modify calc functions WEEKNUM(_ADD) to comply with ODFF1.2
Hi Miguel, SUMIFS 6.16.63, AVERAGEIFS 6.18.6, COUNTIFS 6.13.10 The functions you name wait for license clearance to integrate the OOo code for these functions in LibO. fdo 41214 is about SUMIFS, AVERAGEIF(S) and COUNTIFS, you can read about all the details there. Winfried ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [PATCH] fdo#50950 modify calc functions WEEKNUM(_ADD) to comply with ODFF1.2
Hi Winfried,
Long overdue, but better late than never..
This needed some detailed review, which is why I postponed it
originally.
On Friday, 2012-10-05 12:25:57 +0200, Winfried Donkers wrote:
Attached patch modifies the calc functions WEEKNUM and WEEKNUM_ADD to
respectively ISOWEEKNUM and WEEKNUM. These latter functions comply
with ODFF1.2
Generally the patch works, but there are some things that need
consideration and more work.
While this
--- a/formula/source/core/resource/core_resource.src
+++ b/formula/source/core/resource/core_resource.src
@@ -317,7 +317,7 @@ Resource RID_STRLIST_FUNCTION_NAMES_ENGLISH_ODFF
-String SC_OPCODE_WEEK { Text = ISOWEEKNUM ; };
+String SC_OPCODE_ISOWEEKNUM { Text = ISOWEEKNUM ; };
changing only the identifier is fine, this
@@ -650,7 +650,7 @@ Resource RID_STRLIST_FUNCTION_NAMES_ENGLISH
-String SC_OPCODE_WEEK { Text = WEEKNUM ; };
+String SC_OPCODE_ISOWEEKNUM { Text = ISOWEEKNUM ; };
changing the string WEEKNUM to ISOWEEKNUM does not work, as these
resources are also used to save/load in ODF 1.0/1.1 and compile formulas
set via API. So saving in ODF 1.0/1.1 and reading in an older release
will fail and vice versa.
For connected reason, this
--- a/sc/source/core/tool/odffmap.cxx
+++ b/sc/source/core/tool/odffmap.cxx
@@ -35,7 +35,7 @@ ScCompiler::AddInMap ScCompiler::maAddInMap[] =
-{ WEEKNUM, WEEKNUM_ADD, false,
com.sun.star.sheet.addin.Analysis.getWeeknum, COM.SUN.STAR.SHEET.ADDIN.ANALYS
+{ WEEKNUM, WEEKNUM, false,
com.sun.star.sheet.addin.Analysis.getWeeknum,
COM.SUN.STAR.SHEET.ADDIN.ANALYSIS.G
would not work if the above change is not done as there would be two
WEEKNUM in the English map.
Not changing the strings on the other hand would leave them unchanged in
the use English function names case, which would be confusing for the
user as then we'd have ISOWEEKNUM and WEEKNUM vs. WEEKNUM and
WEEKNUM_ADD with WEEKNUM meaning two different things. I think we could
get around this by patching the generated symbol table for the English
case in the compiler, instead of duplicating
RID_STRLIST_FUNCTION_NAMES_ENGLISH, and add another field in AddInMap,
I could take a look later. For now just don't rename the functions.
Not renaming WEEKNUM_ADD to WEEKNUM also obsoletes this change
--- a/scaddins/source/analysis/analysishelper.cxx
+++ b/scaddins/source/analysis/analysishelper.cxx
@@ -48,7 +48,7 @@ const FuncDataBase pFuncDatas[] =
-FUNCDATA( Weeknum, DOUBLE, INTPAR, 2,
FDCat_DateTime ),
+FUNCDATA( Weeknum, UNIQUE, INTPAR, 2,
FDCat_DateTime ),
The link to the help files has been broken, i.e. WEEKNUM now points to
the (partially out of date) help text for WEEKNUM_ADD, but ISOWEEKNUM
does not point to a help text.
The latter because of
--- a/sc/inc/helpids.h
+++ b/sc/inc/helpids.h
@@ -396,7 +396,7 @@
-#define HID_FUNC_KALENDERWOCHE
SC_HID_FUNC_KALENDERWOCHE
+#define HID_FUNC_ISOWEEKNUM
SC_HID_FUNC_ISOWEEKNUM
SC_HID_FUNC_KALENDERWOCHE is used in
helpcontent2/source/text/scalc/01/func_weeknum.xhp:bookmark xml-lang=en-US
branch=hid/SC_HID_FUNC_KALENDERWOCHE id=bm_id3158403
localize=false/paragraph role=heading id=hd_id3159161 xml-lang=en-US
level=2 l10n=U oldref=54variable id=weeknumlink
href=text/scalc/01/func_weeknum.xhpWEEKNUM/link
that should be adapted. The WEEKNUM_ADD help is in
helpcontent2/source/text/scalc/01/func_weeknumadd.xhp
One important matter is still open:
When opening calc-documents that use the 'old' functions, they are not
always computed correctly. The 'new' functions differ in paramter
count.
I do not know where to add code for backward compatibility when
opening these documents, but with some help/hints I might be able to
fix that.
There are three cases to be considered:
1. reading ODF 1.2 (ODFF)
2. writing ODF 1.0/1.1 (PODF)
3. reading ODF 1.0/1.1 (PODF)
1. Even if ODFF defines number of parameters and whether optional or
not, older releases wrote the WEEKNUM internal function as ISOWEEKNUM
with 2 parameters instead of 1.
* could be checked in formula/source/core/api/FormulaCompiler.cxx
FormulaCompiler::Factor()
if (eOp == ocIsoWeeknum mxSymbols-isODFF())
and if it has 2 parameters put a token for the WEEKNUM Add-In
instead. This would need some poking to mess around with the actual
function token, a simple pFacToken-NewOpCode(...) would not do as
a FormulaExternalToken is needed instead.
2. ISOWEEKNUM needs to be written as WEEKNUM again.
* parameter could be added in formula/source/core/api/token.cxx
MissingConvention::isRewriteNeeded()
FormulaMissingContext::AddMoreArgs()
* not having changed the function name in the resources should
actually write the correct name, to be verified.
3. Very similar to #1, but instead the condition would be
if (eOp == ocIsoWeeknum mxSymbols-isPODF())
So it may
Re: [PATCH] fdo#50950 modify calc functions WEEKNUM(_ADD) to comply with ODFF1.2
Hi Winfried,
On Tuesday, 2012-11-13 11:08:07 +0100, Winfried Donkers wrote:
Would it be an idea if I submit the patch via gerrit? Not to hurry you, but
to prevent it from getting burried accidentally ;)
It's just buried in my mail folder ;-)
Now that I wrote the review we don't need it in gerrit (anymore).
(same goes for IFERROR() and IFNA() patches)
If you have something ready for those you can submit it to gerrit,
that's fine.
By the way, any suggestions/preferences for the next ODFF1.2 formulas to
start working on?
NUMBERVALUE would be good, to have a locale-independent (as opposed to
VALUE) function to convert text to numeric values. Note that some
fragments are already in the code base, but not implemented yet:
grep -r NUMBERVALUE formula/{inc,source} sc/{inc,source}
Btw, in sc/source/ui/src/scfuncs.src the function lacks the parameter
for group separators.
Eike
--
LibreOffice Calc developer. Number formatter stricken i18n transpositionizer.
GnuPG key 0x293C05FD : 997A 4C60 CE41 0149 0DB3 9E96 2F1A D073 293C 05FD
Support the FSFE, care about Free Software! https://fsfe.org/support/?erack
pgpECuaKOzHzg.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
RE: [PATCH] fdo#50950 modify calc functions WEEKNUM(_ADD) to comply with ODFF1.2
Hi Eike. Now that I wrote the review we don't need it in gerrit (anymore). I look forward to your comments and probably will modify the code accordingly ;) (same goes for IFERROR() and IFNA() patches) If you have something ready for those you can submit it to gerrit, that's fine. I will, later today. By the way, any suggestions/preferences for the next ODFF1.2 formulas to start working on? NUMBERVALUE would be good ... Incidentally, I was looking at the same function :) I created bug 57180 for NUMBERVALUE and will start working on it. Winfried ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
RE: [PATCH] fdo#50950 modify calc functions WEEKNUM(_ADD) to comply with ODFF1.2
Hi Eike, Would it be an idea if I submit the patch via gerrit? Not to hurry you, but to prevent it from getting burried accidentally ;) (same goes for IFERROR() and IFNA() patches) By the way, any suggestions/preferences for the next ODFF1.2 formulas to start working on? Winfried -Oorspronkelijk bericht- Van: Eike Rathke [mailto:er...@redhat.com] Verzonden: maandag 15 oktober 2012 14:54 Aan: Winfried Donkers CC: libreoffice@lists.freedesktop.org Onderwerp: Re: [PATCH] fdo#50950 modify calc functions WEEKNUM(_ADD) to comply with ODFF1.2 Hi Winfried, On Friday, 2012-10-05 12:25:57 +0200, Winfried Donkers wrote: Attached patch modifies the calc functions WEEKNUM and WEEKNUM_ADD to respectively ISOWEEKNUM and WEEKNUM. Just to tell you that I didn't forget about this patch, I'll come back to you with details later, maybe only after LibOCon. Thanks Eike -- LibreOffice Calc developer. Number formatter stricken i18n transpositionizer. GnuPG key 0x293C05FD : 997A 4C60 CE41 0149 0DB3 9E96 2F1A D073 293C 05FD Support the FSFE, care about Free Software! https://fsfe.org/support/?erack ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
RE: [PATCH] fdo#50950 modify calc functions WEEKNUM(_ADD) to comply with ODFF1.2
Hi Eike, Is it correct that the patch has been pushed to master? I do not see anything related in master, probably it is just in your local branch? You are quite right, of course :) I apologise for disturbing you and quickly return to my investigating/experimenting with IFERROR(). Winfried ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
RE: [PATCH] fdo#50950 modify calc functions WEEKNUM(_ADD) to comply with ODFF1.2
Attached patch modifies the calc functions WEEKNUM and WEEKNUM_ADD to respectively ISOWEEKNUM and WEEKNUM. I noted that the patch for fdo50950 has been pushed to master whilst 'git pul --rebase'-ing this morning. However i did not see any mail that it has been checked. Is it correct that the patch has been pushed to master? Winfried ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [PATCH] fdo#50950 modify calc functions WEEKNUM(_ADD) to comply with ODFF1.2
Hi Winfried, On Monday, 2012-10-22 09:27:35 +0200, Winfried Donkers wrote: I noted that the patch for fdo50950 has been pushed to master whilst 'git pul --rebase'-ing this morning. However i did not see any mail that it has been checked. Is it correct that the patch has been pushed to master? I do not see anything related in master, probably it is just in your local branch? Eike -- LibreOffice Calc developer. Number formatter stricken i18n transpositionizer. GnuPG key 0x293C05FD : 997A 4C60 CE41 0149 0DB3 9E96 2F1A D073 293C 05FD Support the FSFE, care about Free Software! https://fsfe.org/support/?erack pgpv92P5JFfGE.pgp Description: PGP signature ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [PATCH] fdo#50950 modify calc functions WEEKNUM(_ADD) to comply with ODFF1.2
Hi Winfried, On Friday, 2012-10-05 12:25:57 +0200, Winfried Donkers wrote: Attached patch modifies the calc functions WEEKNUM and WEEKNUM_ADD to respectively ISOWEEKNUM and WEEKNUM. Just to tell you that I didn't forget about this patch, I'll come back to you with details later, maybe only after LibOCon. Thanks Eike -- LibreOffice Calc developer. Number formatter stricken i18n transpositionizer. GnuPG key 0x293C05FD : 997A 4C60 CE41 0149 0DB3 9E96 2F1A D073 293C 05FD Support the FSFE, care about Free Software! https://fsfe.org/support/?erack pgpqvwDpVH1pP.pgp Description: PGP signature ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[PATCH] fdo#50950 modify calc functions WEEKNUM(_ADD) to comply with ODFF1.2
Attached patch modifies the calc functions WEEKNUM and WEEKNUM_ADD to respectively ISOWEEKNUM and WEEKNUM. These latter functions comply with ODFF1.2 The link to the help files has been broken, i.e. WEEKNUM now points to the (partially out of date) help text for WEEKNUM_ADD, but ISOWEEKNUM does not point to a help text. One important matter is still open: When opening calc-documents that use the 'old' functions, they are not always computed correctly. The 'new' functions differ in paramter count. I do not know where to add code for backward compatibility when opening these documents, but with some help/hints I might be able to fix that. Winfried 0001-fdo-50950-modify-calc-functions-WEEKNUM-_ADD-to-comp.patch Description: 0001-fdo-50950-modify-calc-functions-WEEKNUM-_ADD-to-comp.patch ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
