Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autoremove a locale only if its not the fallback for another enabled one #123

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Autoremove a locale only if its not the fallback for another enabled one #123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1a57d31
Introduce Locale::hasFallback to check if a locale is a fallback for …
bzeller Jun 26, 2018
9e36ad9
Autoremove a locale only if its not the fallback for another enabled one
bzeller Jun 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions zypp/Locale.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,15 @@ namespace zypp
std::string Locale::name() const
{ return CodeMaps::instance().name( _str ); }

bool Locale::hasFallback(const Locale &loc_r) const
{
for ( Locale fb = fallback(); fb; fb = fb.fallback() ) {
if ( fb == loc_r )
return true;
}
return false;
}

Locale Locale::fallback() const
{ return CodeMaps::instance().fallback( _str ); }

Expand Down
3 changes: 3 additions & 0 deletions zypp/Locale.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ namespace zypp
/** Return the translated locale name. */
std::string name() const;

/** Checks if \a loc is in the fallback list for this locale */
bool hasFallback (const Locale &loc_r ) const;

public:
/** Return the fallback locale for this locale, if no fallback exists the empty Locale::noCode.
* The usual fallback sequence is "language_COUNTRY" -> "language" -> Locale::enCode ("en")
Expand Down
19 changes: 17 additions & 2 deletions zypp/sat/detail/PoolImpl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,8 +505,23 @@ namespace zypp
// Add removed locales+fallback except they are still in current
for ( Locale lang: localesTracker.removed() )
{
for ( ; lang && ! localeIds.current().count( IdString(lang) ); lang = lang.fallback() )
{ localeIds.removed().insert( IdString(lang) ); }
if ( lang && ! localeIds.current().count( IdString(lang) ) ) {

//remove the requested one
localeIds.removed().insert( IdString(lang) );

//remove fallbacks
const auto &currLangs = localeIds.current();
auto checkIsFallback = [&lang]( const IdString &currLoc_r ){
return Locale(currLoc_r).hasFallback(lang);
};
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 ) )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

localeIds.removed().insert( IdString(lang) );
}
}

}

// Assert that TrackedLocaleIds::current is not empty.
Expand Down