StringUtils: remove legacy fmt::sprintf usage

[lrusak]

This cleans ups the StringUtils::Format method now that all the format strings are updated to the new style.

Removing the sprintf method makes it so that old sprintf formatting won't work anymore! Be aware!

[Montellese]

Do you plan on replacing all calls to StringUtils::Format() with direct calls to fmt::format()? IMO that would make it more clear that you can't use sprintf() style syntax anymore and you would also benefit from other libfmt features like named arguments etc.

Obviously we would need a temporary workaround for those cases where the EnumToInt() hack applies.

[lrusak]

Do you plan on replacing all calls to StringUtils::Format() with direct calls to fmt::format()? IMO that would make it more clear that you can't use sprintf() style syntax anymore and you would also benefit from other libfmt features like named arguments etc.

I've thought about this, I'm not sure yet though if we should just add a try/catch block here to catch any invalid formatting or not.

Obviously we would need a temporary workaround for those cases where the EnumToInt() hack applies.

I'll take a look to see when libfmt started supporting enums and see if we can bump/drop the workaround.

[lrusak]

I'm not really sure why the tests are failing here. I quick look over doesn't reveal anything obvious.

[Montellese]

@lrusak: The problem is located here: https://github.com/xbmc/xbmc/blob/master/xbmc/utils/HttpRangeUtils.cpp#L25. Your regex missed this formatting pattern (because it's not very obvious). Changing that line to use "{}" instead of "%" PRIu64 fixes all failing unit tests on Win64 for me.

I also found https://github.com/xbmc/xbmc/blob/master/xbmc/events/BaseEvent.cpp#L106 by grepping for PRI[a-z]*[0-9]+

[lrusak]

@Montellese thanks for the hints!

[lrusak]

Should we merge this as is? I think the next step would be to remove StringUtils::Format completely.

[Montellese]

For me this is good to go.

[howie-f]

next step would be to remove StringUtils::Format completely

how would we catch exceptions caused by malformed strings of any kind when Format is removed?

[ronie]

@lrusak this is causing some issues, most notably the 'recently added movies' / 'recently added episodes' disappeared from Estuary's home screen.

before this PR

with this PR

Also the file naming of screenshots is broken..

before:
screenshot0000.png

after:
screenshot%05d.png

[fuzzard]

xbmc/xbmc/Util.cpp

Line 716 in 68bf07e

std::string CUtil::GetNextFilename(const std::string &fn_template, int max)

Would be where the screenshot stuff is going wrong. Filecache stuff looks like it'll be affected as well

[ronie]

related errors from the log

2021-05-30 01:24:12.448 T:253782 ERROR : SQL: [MyVideos119.db] SQLite error SQLITE_ERROR (near "%": syntax error)
Query: select * from movie_view WHERE ((%s > '2012-01-01')) AND ((movie_view.playCount IS NULL OR movie_view.playCount < 1))

2021-05-30 01:24:12.464 T:253781 ERROR : SQL: [MyVideos119.db] SQLite error SQLITE_ERROR (near "%": syntax error)
Query: select * from episode_view WHERE ((%s > '2012-01-01')) AND ((episode_view.playCount IS NULL OR episode_view.playCount < 1))

[Montellese]

related errors from the log

2021-05-30 01:24:12.448 T:253782 ERROR : SQL: [MyVideos119.db] SQLite error SQLITE_ERROR (near "%": syntax error)
Query: select * from movie_view WHERE ((%s > '2012-01-01')) AND ((movie_view.playCount IS NULL OR movie_view.playCount < 1))

2021-05-30 01:24:12.464 T:253781 ERROR : SQL: [MyVideos119.db] SQLite error SQLITE_ERROR (near "%": syntax error)
Query: select * from episode_view WHERE ((%s > '2012-01-01')) AND ((episode_view.playCount IS NULL OR episode_view.playCount < 1))

Beware that all SQL queries must use PrepareSQL() and not StringUtils::Format(). While it works as well it foregoes some SQL specific checks (like SQL injections) and workarounds to support both SQLite and MySQL.

[DaveTBlake]

Needless to say that the music smart playlists are also broken by this change too.