Rework formatting for logs and fmt::format#1890
Rework formatting for logs and fmt::format#1890Alphalaneous wants to merge 3 commits intogeode-sdk:mainfrom
Conversation
| geode::Result<std::string> format(cocos2d::CCObject const* obj, std::string_view specifier) override { | ||
| if (!geode::cast::typeinfo_cast<T*>(obj)) return geode::Err("Incorrect Type"); | ||
| if (!m_formatter) return geode::Err("No formatter callback"); | ||
| if (m_formatter) return geode::Ok(m_formatter(static_cast<T*>(const_cast<cocos2d::CCObject*>(obj)), specifier)); |
There was a problem hiding this comment.
the redundant if (m_formatter) here can be a bit confusing so i'd remove it
| cocos2d::CCObject* m_obj; | ||
| }; | ||
|
|
||
| class FormatBase { |
There was a problem hiding this comment.
since you use unique_ptr, I recommend adding a virtual destructor to this class. Most formatters probably won't have any state, but without a virtual destructor you make it so that their destructors never get to run at all which can cause subtle bugs
| }); | ||
|
|
||
| format::registerFormat<CCArray>([] (CCArray* self, std::string_view specifier) -> std::string { | ||
| if (!self && !self->count()) return "[empty]"; |
There was a problem hiding this comment.
this check sounds erroneous, did you mean !self || !self->count()
| }); | ||
|
|
||
| format::registerFormat<CCSet>([] (CCSet* self, std::string_view specifier) -> std::string { | ||
| if (!self && !self->count()) return "[empty]"; |
| } | ||
|
|
||
| void addFormat(std::string_view name, std::unique_ptr<FormatBase> format) { | ||
| m_formatCallbacks.insert(m_formatCallbacks.begin(), std::move(format)); |
There was a problem hiding this comment.
is there a specific reason why this inserts at the beginning (which is going to be O(n) always)? if this is to make new listeners highest priority, IMO just iterate in reverse when calling
| fmt::memory_buffer buffer; | ||
| buffer.push_back('['); | ||
|
|
||
| bool first = true; | ||
|
|
||
| for (auto obj : self->asExt()) { | ||
| if (!first) buffer.append(std::string_view(", ")); | ||
| if (!rawFormat) buffer.append(std::string_view("\n\t")); | ||
| first = false; | ||
|
|
||
| buffer.append(fmt::format("{}", format::wrap(obj))); | ||
| } | ||
|
|
||
| if (!rawFormat) buffer.push_back('\n'); | ||
| buffer.push_back(']'); | ||
| return fmt::to_string(buffer); |
There was a problem hiding this comment.
I would recommend using geode::utils::StringBuffer (in here and all other functions), it's going to be more efficient and less verbose to use (e.g. you can directly do buffer.append(", ") or buffer.append("{}", format::wrap(obj))
2 participants
Separate formatting from the log headers and source
Runtime formatting for CCObjects
fmt::format("My Object: {}", format::wrap(CCString::create("Wow!")));New formats for CCFloat, CCDouble, CCInteger, CCBool, CCString, CCSet, CCDictionary, CCTouch, CCFiniteTimeAction, and CCActionInterval
Prettify container formats by default, use the
rawspecifier to disable prettifying:log::info("{:raw}", this->m_myDict);Note that I did not remove the
format_asfor CCObject, CCArray, and CCNode, even though they are now obsolete. As that would be considered a breaking change. These will still function the same if someone is calling them for any reason.