utils: Don't strip modifiers when stripping encoding #322
Conversation
|
btw, bonus change to replace |
|
Other than the minor changes above, LGTM, thanks @iainlane! |
iainlane
force-pushed
the
strip-locale-keep-modifiers
branch
3 times, most recently
from
1567299 to
6816d54
Compare
|
Merci for the review! |
There was a problem hiding this comment.
Looks good except for the mentioned changes. Since as_gstring_replace is no public API, can you, besides conditionally redirecting it to g_string_replace if that's available, also add the limit parameter and set that to 1 when using it to replace the UTF-8 "sometimes-suffix"? It'll probably not actually improve performance by an amount that matters, given how short the string is, but it's likely still nice to have.
We have some locales which are dupes if we do this, for example ca_ES.UTF-8 and ca_ES.UTF-8@valencia. The latter of these should become ca_ES@valencia in the output. That is what `locale -a` shows. Previously `as_locale_strip_encoding ()` modified the passed-in string in place. However, in the one place where don't `g_strdup ()` the string before passing to this function, it is the key in a `GHashTable`. We can't do this, and only get away with it because the hash table isn't touched after this call. Fix the function to instead return a newly allocated string, and drop the `g_strdup` calls from the other call sites. Add a small test for this function too. ximion/appstream-generator#92
Until we can depend on 2.68, it's probably best to standardise on one implementation - also, this one is much simpler.
iainlane
force-pushed
the
strip-locale-keep-modifiers
branch
from
6816d54 to
8f8327b
Compare
|
All done. Please check the SPDX changes, as I don't know what those input strings are to know if it should be |
| * Performs multiple search and replace operations on the given string. | ||
| * | ||
| * Returns: the number of replacements done, or 0 if @search is not found. | ||
| * g_string_replace: |
There was a problem hiding this comment.
that's a typo (c+p from glib)
We have some locales which are dupes if we do this, for example ca_ES.UTF-8 and ca_ES.UTF-8@valencia. The latter of these should become ca_ES@valencia in the output. That is what
locale -ashows.Previously
as_locale_strip_encoding ()modified the passed-in string in place. However, in the one place where don'tg_strdup ()the string before passing to this function, it is the key in aGHashTable. We can't do this, and only get away with it because the hash table isn't touched after this call. Fix the function to instead return a newly allocated string, and drop theg_strdupcalls from the other call sites.Add a small test for this function too.
ximion/appstream-generator#92