Autoremove a locale only if its not the fallback for another enabled one #123
Conversation
zypp/sat/detail/PoolImpl.cc
Outdated
| const auto &currLangs = localeIds.current(); | ||
| for ( lang = lang.fallback(); lang && ! localeIds.current().count( IdString(lang) ); lang = lang.fallback() ) { | ||
| //remove the language only if its not a fallback for any other | ||
| if ( std::none_of(currLangs.begin(), currLangs.end(), [&lang](const IdString &loc){ return Locale(loc).fallback() == lang; }) ) |
There was a problem hiding this comment.
Fallbacks form a list de_DE->de->en. We don't want to remove lang if it's in the fallbackllist of a selected locale, not only if it's the direct fallback. IMO the code would remove en if currLangs is just de_DE.
I'd enhance the Locale class and add a member bool hasFallback( const Locale & fallback_r ) const; returning whether fallback_r is in the Locales fallback list (separate commit) and use this.
bzeller
force-pushed
the
remove_locale_if_no_fallback
branch
from
cffd83b
to
fb287ef
Compare
bzeller
force-pushed
the
remove_locale_if_no_fallback
branch
from
fb287ef
to
45a98e9
Compare
zypp/sat/detail/PoolImpl.cc
Outdated
| const auto &currLangs = localeIds.current(); | ||
| auto checkIsFallback = [&lang]( const IdString &currLocStr_r ){ | ||
| Locale currLoc(currLocStr_r); | ||
| return ( currLoc && Locale(currLoc).hasFallback(lang) ); |
There was a problem hiding this comment.
Doppeltgemoppelt. Locale currLoc(currLocStr_r); and Locale(currLoc).hasF...
Actually you don't need the extra currLoc variable. Even if currLangs had an empty string included, Locale(currLocStr_r) would be the empty Locale (Locale::noCode) and hasFallback would always be false.
return Locale(currLocStr_r).hasFallback(lang);
bzeller
force-pushed
the
remove_locale_if_no_fallback
branch
from
45a98e9
to
9e36ad9
Compare
| }; | ||
| for ( lang = lang.fallback(); lang && ! localeIds.current().count( IdString(lang) ); lang = lang.fallback() ) { | ||
| //remove the language only if its not a fallback for any other | ||
| if ( std::none_of( currLangs.begin(), currLangs.end(), checkIsFallback ) ) |
There was a problem hiding this comment.
No longer sure if this is the right approach / whether checkIsFallback can actually happen.
The 1st of the 3 for-loops (L490) adds all already supported locales (in localesTracker.current but nor newly added) and their fallbacks to localeIds.current().
The 2nd loop (L499) adds all newly added locales including their fallbacks to localeIds.added(), unless a locale is already in localeIds.current() (in this case it's fallbacks are also already in current()).
IMO the original 3rd-loop is almost right. All removed locales and their fallbacks are remembered in localeIds.removed() unless a fallback is already in localeIds.current() . It just misses an additional check for && ! localeIds.added().count( IdString(lang) ). The fallback must not be removed it was newly added (removed de_DE but added de).
In contrast to localesTracker, localeIds.current() contains all fallbacks. So if a locale is not in current, it can't be fallback of a locale in current.
This PR adds a extra check before automatically removing a locales fallback. It will only be removed if its not a fallback for another requested locale.