Skip to content

Commit

Permalink
I18N: Set C++ locale and format sizes according to the locale
Browse files Browse the repository at this point in the history
Displaying a package size, an installation size, a download speed etc.
formats decimal numbers to 1-digit precision after the decimal point
(49.5 KiB). However, users expect the number to be formatted according
their locale. E.g. in cs_CZ.UTF-8, it is "49,5 KiB".

DNF5 formats these values with fmt::format() which utilizes C++ locale
if "L" formatting option is used.

C++ locale (std::locale::global()) and C locale (setlocale(),
C++-wrapped as std::setlocale()) are two different things and DNF5
only has set the C locale up to now.

This patch starts setting C++ locale, which also implicitly sets
C locale. This patch also modifies
libdnf5::cli::utils::units::format_size_aligned() to use the
locale-dependent decimal seperator (available since fmt-8.0.0).

I manually tested dnf5 and dnf5daemon-client and they work for me.

Though there is a risk that the new C++ locale will affect some code
uknown to me, like regular expression matching, or thread-specific
locales. If the affected code was unfixable, we can resort to saving
the desired C++ locale into a dedicated object accessible to
format_size_aligned() and pass it explicitly for fmt::format. Thorough
testing is welcome.

Related: rpm-software-management#1687
  • Loading branch information
ppisar committed Sep 16, 2024
1 parent 3b64925 commit 4b74503
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 10 deletions.
10 changes: 5 additions & 5 deletions dnf5/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,14 @@ along with libdnf. If not, see <https://www.gnu.org/licenses/>.
#include <libdnf5/utils/bgettext/bgettext-mark-domain.h>
#include <libdnf5/utils/locker.hpp>
#include <libdnf5/version.hpp>
#include <locale.h>
#include <string.h>

#include <algorithm>
#include <cmath>
#include <cstring>
#include <filesystem>
#include <iostream>
#include <locale>

constexpr const char * DNF5_LOGGER_FILENAME = "dnf5.log";

Expand Down Expand Up @@ -940,11 +940,11 @@ static void print_new_leaves(Context & context) {
}

static void set_locale() {
auto * locale = setlocale(LC_ALL, "");
if (locale) {
return;
try {
std::locale::global(std::locale(""));
} catch (std::runtime_error & ex) {
std::cerr << "Failed to set locale, defaulting to \"C\"" << std::endl;
}
std::cerr << "Failed to set locale, defaulting to \"C\"" << std::endl;
}

} // namespace dnf5
Expand Down
8 changes: 6 additions & 2 deletions dnf5daemon-client/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ along with libdnf. If not, see <https://www.gnu.org/licenses/>.
#include <libdnf5/logger/stream_logger.hpp>
#include <libdnf5/utils/bgettext/bgettext-lib.h>
#include <libdnf5/utils/bgettext/bgettext-mark-domain.h>
#include <locale.h>
#include <string.h>

#include <cstring>
#include <filesystem>
#include <fstream>
#include <iostream>
#include <locale>

namespace dnfdaemon::client {

Expand Down Expand Up @@ -220,7 +220,11 @@ static void add_commands(Context & context) {
int main(int argc, char * argv[]) {
std::unique_ptr<sdbus::IConnection> connection;

setlocale(LC_ALL, "");
try {
std::locale::global(std::locale(""));
} catch (std::runtime_error & ext) {
}


dnfdaemon::client::Context context;

Expand Down
3 changes: 3 additions & 0 deletions libdnf5-cli/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ target_link_libraries(libdnf5-cli PRIVATE common)
target_link_libraries(libdnf5-cli PUBLIC libdnf5)

pkg_check_modules(LIBFMT REQUIRED fmt)
if (LIBFMT_VERSION VERSION_LESS 8.0.0)
add_definitions(-DFMT_PRE_8)
endif()
list(APPEND LIBDNF5_CLI_PC_REQUIRES "${LIBFMT_MODULE_NAME}")
target_link_libraries(libdnf5-cli PUBLIC ${LIBFMT_LIBRARIES})

Expand Down
9 changes: 8 additions & 1 deletion libdnf5-cli/utils/units.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,14 @@ std::pair<float, const char *> to_size(int64_t num) {

std::string format_size_aligned(int64_t num) {
auto [value, unit] = to_size(num);
return fmt::format("{0:.1f} {1:>3s}", value, unit);
return fmt::format(
#ifdef FMT_PRE_8
"{0:.1f} {1:>3s}",
#else
"{0:.1Lf} {1:>3s}",
#endif
value,
unit);
}


Expand Down
4 changes: 2 additions & 2 deletions test/libdnf5-cli/utils/test_utf8.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ along with libdnf. If not, see <https://www.gnu.org/licenses/>.

#include "utils/utf8.hpp"

#include <clocale>
#include <locale>


CPPUNIT_TEST_SUITE_REGISTRATION(UTF8Test);


void UTF8Test::setUp() {
// wide characters do not work at all until we set locales in the code
setlocale(LC_ALL, "C.UTF-8");
std::locale::global(std::locale("C.UTF-8"));

hello_world_en = "Hello world!";
hello_world_cs = "Ahoj světe!";
Expand Down

0 comments on commit 4b74503

Please sign in to comment.