Improvement: Add libfmt support

[wutschel]

Description

Closes #495.

This PR adds support for libfmt. It implements the function convert_fmt which converts from fmt to char.

The only place used is when adding a label to Kodi Settings' slider and related custom buttons (e.g. showing "750 ms" instead of "750".

Screenshot:

Summary for release notes

Improvement: Add libfmt support
Improvement: Show unit (e.g. "ms") for Kodi Settings' slider value

[wutschel]

Even though this works I have some concerns:

1. I am not clear on how to check, if fmt format does match the argument list (only int). I saw fmt crashing, if too few arguments are given, and I cannot control what format Kodi sends.
2. Quite a lot of code to just display slider's label as intended.

[wutschel]

@kambala-decapitator, what's your thought on this one?

[kambala-decapitator]

current fmt usage isn't really good. Your wrapper API should be straightforward: (yes, you can mix ObjC and C++)

NSString* fmtFormatted(NSString* format, int value);

and the implementation based on the docs would be something like this:

NSString* fmtFormatted(NSString* format, int value) {
  try {
    const std::string formatted = fmt::format(format.UTF8string, value);
    // not sure if `length` param should be +1 to account for the terminating 0-character
    return [[NSString alloc] initWithBytes:formatted.data() length:formatted.size() encoding:NSUTF8StringEncoding];
  }
  catch (const fmt::format_error& e) { // this catch is optional
    NSLog(@"fmt format error: %s", e.what());
  }
  catch (const std::exception& e) {
    NSLog(@"generic format error: %s", e.what());
  }
  return [NSString stringWithFormat:@"%d %@", value, format];
}

for the ObjC and C++ mix to work, you must use mm file extension instead of cpp.

I am not clear on how to check, if fmt format does match the argument list (only int). I saw fmt crashing, if too few arguments are given, and I cannot control what format Kodi sends.

well, you can't really do it without fmt (e.g. snprintf) either. Btw if you pass more arguments than needed, then it's ok.

P.S. We'll need to update the library to match upstream's 10.2.1: https://github.com/xbmc/xbmc/blob/master/tools/depends/target/fmt/FMT-VERSION

[kambala-decapitator]

better guard against format == nil if such case is possible

[wutschel]

for the ObjC and C++ mix to work, you must use mm file extension instead of cpp.

Just tried this for en empty file. Compiles as .m, but when I rename it to .mm, I get compile errors. Is there any specific flag I need to set, e.g. compiler options?

[kambala-decapitator]

I don't think so. Could you push a branch?

[wutschel]

I don't think so. Could you push a branch?

Sure: https://github.com/wutschel/Official-Kodi-Remote-iOS/tree/test_mm

[wutschel]

With the latest changes around settings menu, now a slider (Settings > Player > Subtitles > Vertical margin) comes up which uses and displays float values ("{:.2f} %}". This lets fmt crash when combining it with an int value. There is more logic needed to process the value in its native form (integer or float).

[kambala-decapitator]

With the latest changes around settings menu, now a slider (Settings > Player > Subtitles > Vertical margin) comes up which uses and displays float values ("{:.2f} %}". This lets fmt crash when combining it with an int value. There is more logic needed to process the value in its native form (integer or float).

do you know that float is expected without looking at the format string?

[wutschel]

do you know that float is expected without looking at the format string?

Working on it. Via the JSON API there is "type" reported as "number" in such case (otherwise it is "integer"). Preparing this as part of a SettingsValuesVC rework which I am currently on. This way I can prepare the logic for this.

[wutschel]

Working on it. Via the JSON API there is "type" reported as "number" in such case (otherwise it is "integer"). Preparing this as part of a SettingsValuesVC rework which I am currently on. This way I can prepare the logic for this.

Did this now locally, in a hacky but proof-of-concept way. I still think adding libfmt just to add some units to the slider's labels is overkill .

Personally, I would just parse for the units (string after "}") in the fmt format string and add this to the std format, e.g.
fmt format -> unit -> std format
"{0:d} ms" -> " ms" -> "%i ms" (example for integer type setting)
"{:.2f} %" -> " %" -> "%.2f %" (example for number type setting)
"{0:d}.0 dB" -> ".0 dB" -> "%i.0 dB" (yes, this fmt format really exists)

[kambala-decapitator]

I don't think so. Could you push a branch?

Sure: https://github.com/wutschel/Official-Kodi-Remote-iOS/tree/test_mm

it took me a while to figure out what's wrong as I wasn't reading compiler errors attentively :) The issue comes from our ConvenienceMacros.h:

/Applications/Xcode-16.4.0.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator18.5.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSUserDefaults.h:151:69: error: expected identifier
  151 | - (NSDictionary<NSString *, id> *)volatileDomainForName:(NSString *)domainName;

/Users/kambala/dev/kodi/Official-Kodi-Remote-iOS/XBMC Remote/ConvenienceMacros.h:83:20: note: expanded from macro 'domainName'
   83 | #define domainName @"local"
      |                    ^

Renaming domainName fixes the issue. That's one more reason to prefer constants over macros ;)

But after that I had another issue: Module 'TargetConditionals' is defined in both ...

The only fix I found is to disable the PCH. The source of the issue is probably related to having Swift code, but it's also possible that it's some Xcode cache issue on my end. Please check if you face it as well.

I have also cleaned the PCH a bit:

diff --git a/XBMC Remote/Kodi Remote-Prefix.pch b/XBMC Remote/Kodi Remote-Prefix.pch
--- a/XBMC Remote/Kodi Remote-Prefix.pch
+++ b/XBMC Remote/Kodi Remote-Prefix.pch
@@ -1,16 +0,4 @@
-//
-// Prefix header for all source files of the 'XBMC Remote' target in the 'XBMC Remote' project
-//
-
-#import <Availability.h>
-
-#ifndef __IPHONE_4_0
-#warning "This project uses features only available in iOS SDK 4.0 and later."
-#endif
+#import <UIKit/UIKit.h>
+#import <Foundation/Foundation.h>
 
 #import "ConvenienceMacros.h"
-
-#ifdef __OBJC__
-    #import <UIKit/UIKit.h>
-    #import <Foundation/Foundation.h>
-#endif

[wutschel]

Great, this works. Will provide a small PR with these changes to prep for future.

[wutschel]

This seems to be too much of a cleanup. After rebasing this PR I found the cpp code added by convert_fmt caused compile errors. The part

-#ifdef __OBJC__
-    #import <UIKit/UIKit.h>
-    #import <Foundation/Foundation.h>
-#endif

is still needed to be keep both .mm and .cpp files compiling.

@kambala-decapitator, ok, if I raise a PR for this to keep supporting these options?

[wutschel]

Added a mm file with a NSString extension to support fmt format with int and float values.

[kambala-decapitator]

ok, if I raise a PR for this to keep supporting these options?

don't see a need for a separate PR, ok to have it in this one