Related: tdf#119613 tdf#118127 suppress duplicated builtins, rework All cat

In number formatter dialog:

Some locales may define duplicated formats, specifically date
formats, for all builtin formats <50 must be present and default
formats may be equal to other formats. Try to suppress duplicated
formats, preferring those that are essential (i.e. used as
current, default or edit formats). This is done only per category,
not in the All category.

Example is [en-ZA] English (South Africa) that now has ISO 8601
also as default date+time format, though there only the date+time
format is suppressed because for date we want to keep both the
locale's default and the fixed ISO format pickable.

The All category still listed the since long deprecated old
currency formats including the worrying CCC format that follows
the current default currency. Eliminate that crap, unless one
matches the current format.

Also, additional formats defined in locale data and user defined
formats were never listed in the All category, do that.

Change-Id: I6e6b9bfecc34d51e89930f348f7afbe5e3c41738
Reviewed-on: https://gerrit.libreoffice.org/61275
Reviewed-by: Eike Rathke <erack@redhat.com>
Tested-by: Eike Rathke <erack@redhat.com>

2 files changed, 179 insertions, 104 deletions

diff --git a/include/svx/numfmtsh.hxx b/include/svx/numfmtsh.hxx
index 3cecade46e68..18485b2ed3a2 100644
--- a/include/svx/numfmtsh.hxx
+++ b/include/svx/numfmtsh.hxx
@@ -197,15 +197,20 @@ private:
     bool                    bIsDefaultValNum;
 
     SVX_DLLPRIVATE short FillEntryList_Impl( std::vector<OUString>& rList );
-    SVX_DLLPRIVATE void  FillEListWithStd_Impl( std::vector<OUString>& rList,sal_uInt16 aPrivCat, short &Pos);
+    SVX_DLLPRIVATE void  FillEListWithStd_Impl( std::vector<OUString>& rList, SvNumFormatType eCategory, short &Pos,
+                                                bool bSuppressDuplicates = false );
     SVX_DLLPRIVATE short FillEListWithFormats_Impl( std::vector<OUString>& rList,short nSelPos,
                                                        NfIndexTableOffset eOffsetStart,
-                                                       NfIndexTableOffset eOffsetEnd);
-    SVX_DLLPRIVATE short FillEListWithDateTime_Impl( std::vector<OUString>& rList,short nSelPos);
+                                                       NfIndexTableOffset eOffsetEnd,
+                                                       bool bSuppressDuplicates );
+    SVX_DLLPRIVATE short FillEListWithDateTime_Impl( std::vector<OUString>& rList,short nSelPos,
+                                                     bool bSuppressDuplicates );
     SVX_DLLPRIVATE short FillEListWithCurrency_Impl( std::vector<OUString>& rList,short nSelPos);
     SVX_DLLPRIVATE short FillEListWithSysCurrencys( std::vector<OUString>& rList,short nSelPos);
     SVX_DLLPRIVATE short FillEListWithUserCurrencys( std::vector<OUString>& rList,short nSelPos);
-    SVX_DLLPRIVATE short FillEListWithUsD_Impl( std::vector<OUString>& rList, sal_uInt16 nPrivCat, short Pos );
+    SVX_DLLPRIVATE short FillEListWithUsD_Impl( std::vector<OUString>& rList, SvNumFormatType eCategory, short Pos );
+
+    SVX_DLLPRIVATE bool IsEssentialFormat_Impl( SvNumFormatType eType, sal_uInt32 nKey );
 
     SVX_DLLPRIVATE ::std::vector<sal_uInt32>::iterator GetRemoved_Impl( size_t nKey );
     SVX_DLLPRIVATE bool                                IsRemoved_Impl( size_t nKey );
diff --git a/svx/source/items/numfmtsh.cxx b/svx/source/items/numfmtsh.cxx
index e99e6ec9712c..8d8547fce33b 100644
--- a/svx/source/items/numfmtsh.cxx
+++ b/svx/source/items/numfmtsh.cxx
@@ -553,105 +553,126 @@ short SvxNumberFormatShell::FillEntryList_Impl( std::vector<OUString>& rList )
      * the list position of the current format. If the list is empty
      * or if there is no current format, SELPOS_NONE is delivered.
      */
-    short nSelPos=0;
-    sal_uInt16 nPrivCat = CAT_CURRENCY;
-    nSelPos=SELPOS_NONE;
+    short nSelPos = SELPOS_NONE;
 
     aCurEntryList.clear();
 
-    if(nCurCategory==SvNumFormatType::ALL)
+    if (nCurCategory == SvNumFormatType::ALL)
     {
-        FillEListWithStd_Impl(rList,CAT_NUMBER,nSelPos);
-        FillEListWithStd_Impl(rList,CAT_PERCENT,nSelPos);
-        FillEListWithStd_Impl(rList,CAT_CURRENCY,nSelPos);
-        FillEListWithStd_Impl(rList,CAT_DATE,nSelPos);
-        FillEListWithStd_Impl(rList,CAT_TIME,nSelPos);
-        FillEListWithStd_Impl(rList,CAT_SCIENTIFIC,nSelPos);
-        FillEListWithStd_Impl(rList,CAT_FRACTION,nSelPos);
-        FillEListWithStd_Impl(rList,CAT_BOOLEAN,nSelPos);
-        FillEListWithStd_Impl(rList,CAT_TEXT,nSelPos);
+        FillEListWithStd_Impl( rList, SvNumFormatType::NUMBER, nSelPos);
+        nSelPos = FillEListWithUsD_Impl( rList, SvNumFormatType::NUMBER, nSelPos);
+
+        FillEListWithStd_Impl( rList, SvNumFormatType::PERCENT, nSelPos);
+        nSelPos = FillEListWithUsD_Impl( rList, SvNumFormatType::PERCENT, nSelPos);
+
+        FillEListWithStd_Impl( rList, SvNumFormatType::CURRENCY, nSelPos);
+        // No FillEListWithUsD_Impl() here, user defined currency formats
+        // were already added.
+
+        FillEListWithStd_Impl( rList, SvNumFormatType::DATE, nSelPos);
+        nSelPos = FillEListWithUsD_Impl( rList, SvNumFormatType::DATE, nSelPos);
+
+        FillEListWithStd_Impl( rList, SvNumFormatType::TIME, nSelPos);
+        nSelPos = FillEListWithUsD_Impl( rList, SvNumFormatType::TIME, nSelPos);
+
+        nSelPos = FillEListWithDateTime_Impl( rList, nSelPos, false);
+        nSelPos = FillEListWithUsD_Impl( rList, SvNumFormatType::DATETIME, nSelPos);
+
+        FillEListWithStd_Impl( rList, SvNumFormatType::SCIENTIFIC, nSelPos);
+        nSelPos = FillEListWithUsD_Impl( rList, SvNumFormatType::SCIENTIFIC, nSelPos);
+
+        FillEListWithStd_Impl( rList, SvNumFormatType::FRACTION, nSelPos);
+        nSelPos = FillEListWithUsD_Impl( rList, SvNumFormatType::FRACTION, nSelPos);
+
+        FillEListWithStd_Impl( rList, SvNumFormatType::LOGICAL, nSelPos);
+        nSelPos = FillEListWithUsD_Impl( rList, SvNumFormatType::LOGICAL, nSelPos);
+
+        FillEListWithStd_Impl( rList, SvNumFormatType::TEXT, nSelPos);
+        nSelPos = FillEListWithUsD_Impl( rList, SvNumFormatType::TEXT, nSelPos);
     }
     else
     {
-        CategoryToPos_Impl(nCurCategory, nPrivCat);
-        FillEListWithStd_Impl(rList,nPrivCat,nSelPos);
+        FillEListWithStd_Impl( rList, nCurCategory, nSelPos, true);
+        if (nCurCategory != SvNumFormatType::CURRENCY)
+            nSelPos = FillEListWithUsD_Impl( rList, nCurCategory, nSelPos);
+        if (nCurCategory == SvNumFormatType::DATE || nCurCategory == SvNumFormatType::TIME)
+            nSelPos = FillEListWithDateTime_Impl( rList, nSelPos, true);
     }
 
-    if( nPrivCat!=CAT_CURRENCY)
-        nSelPos=FillEListWithUsD_Impl(rList,nPrivCat,nSelPos);
-
     return nSelPos;
 }
 
 void SvxNumberFormatShell::FillEListWithStd_Impl( std::vector<OUString>& rList,
-                                                  sal_uInt16 nPrivCat,short &nSelPos )
+                                                  SvNumFormatType eCategory, short &nSelPos,
+                                                  bool bSuppressDuplicates )
 {
     /* Create a current list of format entries. The return value is
      * the list position of the current format. If the list is empty
      * or if there is no current format, SELPOS_NONE is delivered.
      */
-    DBG_ASSERT( pCurFmtTable != nullptr, "unknown NumberFormat" );
+
+    assert( pCurFmtTable != nullptr );
 
     aCurrencyFormatList.clear();
 
-    if(nPrivCat==CAT_CURRENCY)
+    NfIndexTableOffset eOffsetStart;
+    NfIndexTableOffset eOffsetEnd;
+
+    switch (eCategory)
     {
-        nSelPos=FillEListWithCurrency_Impl(rList,nSelPos);
+        case SvNumFormatType::NUMBER:
+            eOffsetStart = NF_NUMBER_START;
+            eOffsetEnd = NF_NUMBER_END;
+        break;
+        case SvNumFormatType::PERCENT:
+            eOffsetStart = NF_PERCENT_START;
+            eOffsetEnd = NF_PERCENT_END;
+        break;
+        case SvNumFormatType::CURRENCY:
+            // Currency entries are generated and assembled, ignore
+            // bSuppressDuplicates.
+            nSelPos = FillEListWithCurrency_Impl( rList, nSelPos);
+            return;
+        case SvNumFormatType::DATE:
+            eOffsetStart = NF_DATE_START;
+            eOffsetEnd = NF_DATE_END;
+        break;
+        case SvNumFormatType::TIME:
+            eOffsetStart = NF_TIME_START;
+            eOffsetEnd = NF_TIME_END;
+        break;
+        case SvNumFormatType::SCIENTIFIC:
+            eOffsetStart = NF_SCIENTIFIC_START;
+            eOffsetEnd = NF_SCIENTIFIC_END;
+        break;
+        case SvNumFormatType::FRACTION:
+            eOffsetStart = NF_FRACTION_START;
+            eOffsetEnd = NF_FRACTION_END;
+            // Fraction formats are internally generated by the number
+            // formatter and are not supposed to contain duplicates anyway.
+            nSelPos = FillEListWithFormats_Impl( rList, nSelPos, eOffsetStart, eOffsetEnd, false);
+            nSelPos = FillEListWithFormats_Impl( rList, nSelPos, NF_FRACTION_3D, NF_FRACTION_100, false);
+            return;
+        case SvNumFormatType::LOGICAL:
+            eOffsetStart = NF_BOOLEAN;
+            eOffsetEnd = NF_BOOLEAN;
+        break;
+        case SvNumFormatType::TEXT:
+            eOffsetStart = NF_TEXT;
+            eOffsetEnd = NF_TEXT;
+        break;
+        default:
+            return;
     }
-    else
-    {
-        NfIndexTableOffset eOffsetStart;
-        NfIndexTableOffset eOffsetEnd;
 
-        switch(nPrivCat)
-        {
-            case CAT_NUMBER         :eOffsetStart=NF_NUMBER_START;
-                                     eOffsetEnd=NF_NUMBER_END;
-                                     break;
-            case CAT_PERCENT        :eOffsetStart=NF_PERCENT_START;
-                                     eOffsetEnd=NF_PERCENT_END;
-                                     break;
-            case CAT_CURRENCY       :eOffsetStart=NF_CURRENCY_START;
-                                     eOffsetEnd=NF_CURRENCY_END;
-                                     break;
-            case CAT_DATE           :eOffsetStart=NF_DATE_START;
-                                     eOffsetEnd=NF_DATE_END;
-                                     break;
-            case CAT_TIME           :eOffsetStart=NF_TIME_START;
-                                     eOffsetEnd=NF_TIME_END;
-                                     break;
-            case CAT_SCIENTIFIC     :eOffsetStart=NF_SCIENTIFIC_START;
-                                     eOffsetEnd=NF_SCIENTIFIC_END;
-                                     break;
-            case CAT_FRACTION       :eOffsetStart=NF_FRACTION_START;
-                                     eOffsetEnd=NF_FRACTION_END;
-                                     nSelPos = FillEListWithFormats_Impl( rList, nSelPos, eOffsetStart, eOffsetEnd);
-                                     nSelPos = FillEListWithFormats_Impl( rList, nSelPos, NF_FRACTION_3D, NF_FRACTION_100);
-                                     return;
-            case CAT_BOOLEAN        :eOffsetStart=NF_BOOLEAN;
-                                     eOffsetEnd=NF_BOOLEAN;
-                                     break;
-            case CAT_TEXT           :eOffsetStart=NF_TEXT;
-                                     eOffsetEnd=NF_TEXT;
-                                     break;
-            default                 :return;
-        }
-
-        nSelPos=FillEListWithFormats_Impl(rList,nSelPos,eOffsetStart,eOffsetEnd);
-
-        if(nPrivCat==CAT_DATE || nPrivCat==CAT_TIME)
-        {
-            nSelPos=FillEListWithDateTime_Impl(rList,nSelPos);
-            nSelPos = FillEListWithFormats_Impl( rList, nSelPos,
-                    NF_DATETIME_ISO_YYYYMMDD_HHMMSS, NF_DATETIME_ISO_YYYYMMDD_HHMMSS);
-        }
-    }
+    nSelPos = FillEListWithFormats_Impl( rList, nSelPos, eOffsetStart, eOffsetEnd, bSuppressDuplicates);
 }
 
 short SvxNumberFormatShell::FillEListWithFormats_Impl( std::vector<OUString>& rList,
                                                        short nSelPos,
                                                        NfIndexTableOffset eOffsetStart,
-                                                       NfIndexTableOffset eOffsetEnd)
+                                                       NfIndexTableOffset eOffsetEnd,
+                                                       bool bSuppressDuplicates )
 {
     /* Create a current list of format entries. The return value is
      * the list position of the current format. If the list is empty
@@ -681,15 +702,19 @@ short SvxNumberFormatShell::FillEListWithFormats_Impl( std::vector<OUString>& rL
             nSelPos = ( !IsRemoved_Impl( nNFEntry ) ) ? aCurEntryList.size() : SELPOS_NONE;
         }
 
-        rList.push_back( aNewFormNInfo );
-        aCurEntryList.push_back( nNFEntry );
+        if (!bSuppressDuplicates || IsEssentialFormat_Impl( nMyCat, nNFEntry) ||
+                    std::find( rList.begin(), rList.end(), aNewFormNInfo) == rList.end())
+        {
+            rList.push_back( aNewFormNInfo );
+            aCurEntryList.push_back( nNFEntry );
+        }
     }
 
     return nSelPos;
 }
 
 short SvxNumberFormatShell::FillEListWithDateTime_Impl( std::vector<OUString>& rList,
-                                                        short nSelPos)
+                                                        short nSelPos, bool bSuppressDuplicates )
 {
     sal_uInt16  nMyType;
 
@@ -712,14 +737,55 @@ short SvxNumberFormatShell::FillEListWithDateTime_Impl( std::vector<OUString>& r
                 nSelPos = ( !IsRemoved_Impl( nNFEntry ) ) ? aCurEntryList.size() : SELPOS_NONE;
             }
 
-            rList.push_back( aNewFormNInfo );
-            aCurEntryList.push_back( nNFEntry );
+            if (!bSuppressDuplicates || IsEssentialFormat_Impl( nMyCat, nNFEntry) ||
+                        std::find( rList.begin(), rList.end(), aNewFormNInfo) == rList.end())
+            {
+                // Ugly hack to suppress an ISO date+time format that is the
+                // default date+time format of the locale and identical to the
+                // internally generated one to be added below.
+                if (!bSuppressDuplicates || aNewFormNInfo != "YYYY-MM-DD HH:MM:SS")
+                {
+                    rList.push_back( aNewFormNInfo );
+                    aCurEntryList.push_back( nNFEntry );
+                }
+            }
         }
     }
 
+    // Always add the internally generated ISO format.
+    nSelPos = FillEListWithFormats_Impl( rList, nSelPos,
+            NF_DATETIME_ISO_YYYYMMDD_HHMMSS, NF_DATETIME_ISO_YYYYMMDD_HHMMSS, false);
+
     return nSelPos;
 }
 
+bool SvxNumberFormatShell::IsEssentialFormat_Impl( SvNumFormatType eType, sal_uInt32 nKey )
+{
+    if (nKey == nCurFormatKey)
+        return true;
+
+    const NfIndexTableOffset nIndex = pFormatter->GetIndexTableOffset(nKey);
+    switch (nIndex)
+    {
+        // These are preferred or edit formats.
+        case NF_DATE_SYS_DDMMYYYY:
+        case NF_DATE_ISO_YYYYMMDD:
+        case NF_TIME_HH_MMSS:
+        case NF_TIME_MMSS00:
+        case NF_TIME_HH_MMSS00:
+        case NF_DATETIME_SYS_DDMMYYYY_HHMMSS:
+        case NF_DATETIME_ISO_YYYYMMDD_HHMMSS:
+            return true;
+        default:
+            ;   // nothing
+    }
+
+    if (nKey == pFormatter->GetStandardFormat( eType, eCurLanguage))
+        return true;
+
+    return false;
+}
+
 short SvxNumberFormatShell::FillEListWithCurrency_Impl( std::vector<OUString>& rList,
                                                         short nSelPos)
 {
@@ -1023,48 +1089,52 @@ short SvxNumberFormatShell::FillEListWithUserCurrencys( std::vector<OUString>& r
 
 
 short SvxNumberFormatShell::FillEListWithUsD_Impl( std::vector<OUString>& rList,
-                                                   sal_uInt16 nPrivCat, short nSelPos )
+                                                   SvNumFormatType eCategory, short nSelPos )
 {
     /* Create a current list of format entries. The return value is
      * the list position of the current format. If the list is empty
      * or if there is no current format, SELPOS_NONE is delivered.
      */
-    sal_uInt16 nMyType;
 
-    DBG_ASSERT( pCurFmtTable != nullptr, "unknown NumberFormat" );
+    assert( pCurFmtTable != nullptr );
 
     OUString        aNewFormNInfo;
 
-    bool            bAdditional = (nPrivCat != CAT_USERDEFINED &&
-                                    nCurCategory != SvNumFormatType::ALL);
+    const bool bCatDefined = (eCategory == SvNumFormatType::DEFINED);
+    const bool bCategoryMatch = (eCategory != SvNumFormatType::ALL && !bCatDefined);
 
     for( SvNumberFormatTable::const_iterator it = pCurFmtTable->begin(), aEnd = pCurFmtTable->end(); it != aEnd; ++it )
     {
-        sal_uInt32 nKey = it->first;
         const SvNumberformat* pNumEntry = it->second;
 
+        if (bCategoryMatch && pNumEntry->GetMaskedType() != eCategory)
+            continue;   // for; type does not match category if not ALL
+
+        const bool bUserDefined = bool(pNumEntry->GetType() & SvNumFormatType::DEFINED);
+        if (!bUserDefined && bCatDefined)
+            continue;   // for; not user defined in DEFINED category
+
+        if (!(bUserDefined || (!bCatDefined && pNumEntry->IsAdditionalBuiltin())))
+            continue;   // for; does not match criteria at all
+
+        const sal_uInt32 nKey = it->first;
         if ( !IsRemoved_Impl( nKey ) )
         {
-            if( (pNumEntry->GetType() & SvNumFormatType::DEFINED) ||
-                    (bAdditional && pNumEntry->IsAdditionalBuiltin()) )
-            {
-                SvNumFormatType nMyCat=pNumEntry->GetMaskedType();
-                CategoryToPos_Impl(nMyCat,nMyType);
-                aNewFormNInfo=  pNumEntry->GetFormatstring();
+            aNewFormNInfo = pNumEntry->GetFormatstring();
 
-                bool bFlag=true;
-                if(pNumEntry->HasNewCurrency())
-                {
-                    bool bTestBanking;
-                    sal_uInt16 nPos=FindCurrencyTableEntry(aNewFormNInfo,bTestBanking);
-                    bFlag=!IsInTable(nPos,bTestBanking,aNewFormNInfo);
-                }
-                if(bFlag)
-                {
-                    if ( nKey == nCurFormatKey ) nSelPos = aCurEntryList.size();
-                    rList.push_back( aNewFormNInfo );
-                    aCurEntryList.push_back( nKey );
-                }
+            bool bAdd = true;
+            if (pNumEntry->HasNewCurrency())
+            {
+                bool bTestBanking;
+                sal_uInt16 nPos = FindCurrencyTableEntry(aNewFormNInfo,bTestBanking);
+                bAdd = !IsInTable(nPos,bTestBanking,aNewFormNInfo);
+            }
+            if (bAdd)
+            {
+                if (nKey == nCurFormatKey)
+                    nSelPos = aCurEntryList.size();
+                rList.push_back( aNewFormNInfo );
+                aCurEntryList.push_back( nKey );
             }
         }
     }