dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed.
Oh boy this is going to really give a big conflict with D26202 <https://phabricator.kde.org/D26202>! Given that this commit is "more trivial" than D26202 <https://phabricator.kde.org/D26202>, maybe it should happen again *after* D26202 <https://phabricator.kde.org/D26202> lands... INLINE COMMENTS > anthonyfieroni wrote in kconfigtest.cpp:533-542 > When it has only strings you can use `R"()"` > https://en.cppreference.com/w/cpp/language/string_literal What would be the benefit here? One double-backslash simplified to a single one, and that's it, right? > kconfig_compiler.cpp:694 > defaultValue = QLatin1String("default") + name; > - > + cpp.flush(); > } else if (type == QLatin1String("Color") && !defaultValue.isEmpty()) { Why? Surely the QTextStream destructor (which is called here) will take care of that. > kconfig_compiler.cpp:729 > defaultValue = QLatin1String("default") + name; > + cpp.flush(); > } (same) > kconfig_compiler.cpp:881 > } else if (name.contains(' ')) { > - cout << "Entry '" << name << "' contains spaces! <name> elements can > not contain spaces!" << endl; > + std::cout << "Entry '" << qPrintable(name) << "' contains spaces! > <name> elements can not contain spaces!" << std::endl; > name.remove(' '); (pre-existing) Strange that this one is cout and not cerr... > kconfig_compiler.cpp:1379 > + out << ";\n"; > + out.flush(); > return result; I've been wondering if this flush() is needed. In my tests, it works without. http://www.davidfaure.fr/2020/qtextstream_flush.cpp QTextStream is notified of the QString being deleted, and flushes. But in theory local variables are deleted *after* the return value is copied (otherwise mutex lockers wouldn't make sense, stack overflow confirms) so in the absence of named-return-value-optimization (which IIRC is only guaranteed in C++17 and later), the copying could happen before the flushing. Unless I missed something, let's keep the `flush()` to be safe :-) REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D26432 To: mlaurent, dfaure, tcanabrava Cc: anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
