[REQUEST] Continued implementation of fmtlib

[aristocratos]

Continued implementation of https://github.com/fmtlib/fmt for better string building readability. Should also make some of the custom string functions redundant.

Help is appreciated since there is a lot of string concatenations to convert.

Example of how to format short strings:

out = fmt::format("{0} Text: {1:<10} + {2:>15.15}", Theme::c("main_fg"), left_aligned, right_aligned_clipped);

Example of how to format longer strings with repeated variables:

out_string = fmt::format(
    "{clear}{bg_black}{fg_white}" // Formatting string split up per output line
    "{mv1}Terminal size too small:"
    "{mv2} Width = {fg_width}{width:4d} {fg_white}Height = {fg_height}{height:4d}"
    "{mv3}{fg_white}Needed for current config:"
    "{mv4}Width = {minWidth:4d} Height = {minHeight:4d}",
    // Repeated arguments first
    "clear"_a = Term::clear, "bg_black"_a = Global::bg_black, "fg_white"_a = Global::fg_white,
    // Then using cursor placements as "scope"
    "mv1"_a = Mv::to((height / 2) - 2, (width / 2) - 11),
    "mv2"_a = Mv::to((height / 2) - 1, (width / 2) - 10),
        "fg_width"_a = (width < minWidth ? Global::fg_red : Global::fg_green),
        "width"_a = width,
        "fg_height"_a = (height < minHeight ? Global::fg_red : Global::fg_green),
        "height"_a = height,
    "mv3"_a = Mv::to((height / 2) + 1, (width / 2) - 12),
    "mv4"_a = Mv::to((height / 2) + 2, (width / 2) - 10),
        "minWidth"_a = minWidth,
        "minHeight"_a = minHeight
);

fmtlib reference: https://hackingcpp.com/cpp/libs/fmt.html

Some examples of formatting options:

String options:

Floating-Point options:

Signed Integer options:

Unsigned Integer options:

[imwints]

You should think about adding a .clang-format file to the project, so the multi-line statements like

out_string = format(
    "{clear}{bg_black}{fg_white}" // Formatting string split up per output line
    "{mv1}Terminal size too small:"
    "{mv2} Width = {fg_width}{width:4d} {fg_white}Height = {fg_height}{height:4d}"
    "{mv3}{fg_white}Needed for current config:"
    "{mv4}Width = {minWidth:4d} Height = {minHeight:4d}",
    // Repeated arguments first
    "clear"_a = Term::clear, "bg_black"_a = Global::bg_black, "fg_white"_a = Global::fg_white,
    // Then using cursor placements as "scope"
    "mv1"_a = Mv::to((height / 2) - 2, (width / 2) - 11),
    "mv2"_a = Mv::to((height / 2) - 1, (width / 2) - 10),
        "fg_width"_a = (width < minWidth ? Global::fg_red : Global::fg_green),
        "width"_a = width,
        "fg_height"_a = (height < minHeight ? Global::fg_red : Global::fg_green),
        "height"_a = height,
    "mv3"_a = Mv::to((height / 2) + 1, (width / 2) - 12),
    "mv4"_a = Mv::to((height / 2) + 2, (width / 2) - 10),
        "minWidth"_a = minWidth,
        "minHeight"_a = minHeight
);

are formatted correctly by everyone without the need to add manual line breaks.

[VA1DER]

Not a fan of the switch to fmt. A non-standard library with a requirement for cmake to build makes life difficult for building btop on smaller devices. Perhaps enough of fmt can be brought into your source tree and incorporated with your makefile system that brop won't require a non-standard library to build?

[aristocratos]

@VA1DER If you git clone --recursive https://github.com/aristocratos/btop.git and then compile you'll be happy to notice that, no, you don't need cmake to compile. We are passing a flag for header only.

https://github.com/aristocratos/btop/blob/468993817ad62155631442307f5bdb945a7214f9/src/btop_tools.hpp#L41-L45

@nobounce Not sure how much a .clang-format file would help in this case since we are using a custom function Mv::to() to move the cursor around, and that variable could be named anything. So there is really no indication of a what should be a line break, it's up to the programmers discretion to show what will be printed on different lines.

My example above was just an example. As long as the fmt strings is somewhat readable I'm happy.

[imwints]

@aristocratos

Not sure how much a .clang-format file would help in this case since we are using a custom function Mv::to() to move the > cursor around, and that variable could be named anything.

I don't really understand what you're saying but this is probably the wrong issue to discuss this.

Is there any reason against using the standards std::format? It doesn't have all features included in fmt but is implemented in GCC since version 13 and Clang since version 14...

[aristocratos]

@nobounce

I don't really understand what you're saying but this is probably the wrong issue to discuss this.

You suggested adding a .clang-format file above 👀 Just saying autoformatting the following part wouldn't be possible since the only thing indicating a line break is the variable {mvX} which could be named anything.

"{clear}{bg_black}{fg_white}" // Formatting string split up per output line
"{mv1}Terminal size too small:"
"{mv2} Width = {fg_width}{width:4d} {fg_white}Height = {fg_height}{height:4d}"
"{mv3}{fg_white}Needed for current config:"
"{mv4}Width = {minWidth:4d} Height = {minHeight:4d}"

Regarding std::format, requiring gcc13 or clang16 would make it harder to compile since those aren't easily available in many distros. So it's mostly a question of not decreasing availability.

[imwints]

@aristocratos

You suggested adding a .clang-format file above eyes

Yes I did, but I realised we should keep this issue about fmt. Maybe I'll open an issue and annoy you over there 😇

Regarding std::format, requiring gcc13 or clang16 would make it harder to compile since those aren't easily available in > many distros. So it's mostly a question of not decreasing availability.

That's right, I'll live on the edge with my distros so I hardly notice that some other distros take some time to include newer packages. It might be an option to enable std::format based on the compiler version and use fmt only for compatibility until the vast majority of distros have a GCC 13 and then drop fmt. That has the advantage that we cant test std::format right now. However that would require some additional code which might be undesirable.

[aristocratos]

@nobounce

Yes I did, but I realised we should keep this issue about fmt. Maybe I'll open an issue and annoy you over there 😇

That's no problem, I would say it's related to this issue and suggestions are always welcome :)

That's right, I'll live on the edge with my distros so I hardly notice that some other distros take some time to include newer packages. It might be an option to enable std::format based on the compiler version and use fmt only for compatibility until the vast majority of distros have a GCC 13 and then drop fmt. That has the advantage that we cant test std::format right now. However that would require some additional code which might be undesirable.

Implementing that would probably not be that hard, some macro checks and namespace fmt = std would probably work. I'm more concerned about having two different libraries that don't have 100% feature parity, limiting use of the former and making issues in the latter hard to detect if not compiling with the latest compiler.

[imwints]

@aristocratos I agree with @VA1DER here, not the cmake part, but relying on a third party library should be avoided with std::format making it's why in the standard library. In my opinion we should stick to the standard library and only use fmt as a compat solution for older compilers until we don't need it any longer.

[aristocratos]

@nobounce Just noticed that std::format don't support named arguments. That would be a big reason not to use it over fmtlib, since that's really what make the string formatting readable when having big strings with a lot of variables like in this project.

[imwints]

@aristocratos I've created a sample .clang-format if you want to have a look at it. Here is a diff. It follows the code style used by btop.cpp and mostly changes indentation (Spaces -> Tabs).