On Mon Jul 25 15:31:45 2022 +0000, Piotr Caban wrote:
Have you considered something like in attached patch instead of introducing %S format? I think it's much more robust and I hope it will be possible to remove all hacks/special handling this way (probably except of end of templates case (> >)). The general idea is to introduce OPTIONAL_WS that is used to insert space in all the places where it's optional. The space is added only if first string ends with OPTIONAL_WS and second starts with OPTIONAL_WS. The attached patch was not really tested, it's just a quick WIP version to demonstrate the idea. [tmp.diff](/uploads/399b0a804f25b1c9adb6e820ba309429/tmp.diff)
Hi Piotr,
the good side: it seems to be a step into the right direction (after adding a couple of missed items - not in wine regression list, though, --they should be added--), this seems to output things fine
so perhaps, we can avoid adding ws only discrepancies in regression testing (*)
what it shows (compared to current implementation): - current implement relies on string concatenation and hard coding spaces (with a couple of issues, and trying to patch things) - introducing a state (pending WS '\1') in the output buffer seems an interesting idea and helps contextualize when ws are needed or not
*BUT* there are a bunch of things I don't like in the solution: - inserting optional WS both on LHS and RHS of a same word doesn't seem right - it looks like the dual LHS/RHS have been inserted to handle the '*' case (with either a space before in most of the cases, and sometimes not) - and even defining a keyword with OPTIONAL_WS doesn't seem right (whatever the side)
my gut feeling is that native code looks similar to (in C++)
``` std:stringstream sstr;
if (...) sstr << "const" << undname::ws; else if (...) sstr << "volatile" << undname::ws; else if (...) sstr << "const volatile" << undname::ws;
//
if (...) sstr << "*" << undname::ws; ```
where undname::ws should an iomanip for handling a pending white space separator
in other words: - the control of ws should be attached to stream output/handling functions, not the keyword them selves - it requires to store a pending ws state to be resolved later on, depending on incoming output
here's an attempt to get closer to the above: - derived from your proposal - it only uses RHS optional white spaces - not all possible cleanups have been done - still WIP at this stage - using a trailing '\1' to mark the str_print internal buffer with potential trailing space, and next inserted character is used to decide how to handle the '\1'
[err](/uploads/f1bc1e97dec61387a09ea216d641011f/err)
I'm still unhappy with - the new operator %t to cancel pending ws => still have to test whether moving the optional ws inserting to callers solve the issue
so I'd rather go for a solution where: - '\1' trailing in strings (internal to str_printf) is here to mark optinal WS - callers should use (say '%w') to insert an optional ws
comments welcomed
(*) this one shows also bogus output (note the ' ' after the const and before the ',') (and I have other template examples with 'T const' parameters not generating the extra space... sigh)
?meth1@ZZZ@?$meth2@V?$YYY@V?$WWW@DU?$XXXs@D@RRR@@V?$UUU@D@2@@RRR@@PAVVVV@TTT@@U?$less@V?$WWW@DU?$XXX@D@RRR@@V?$UUU@D@2@@RRR@@@2@V?$UUU@U?$pair@$$CBV?$WWW@DU?$XXX@D@RRR@@V?$UUU@D@2@@std@@PAVVVV@TTT@@@RRR@@@2@$0A@@RRR@@@RRR@@QAEXXZ public: void __thiscall RRR::meth2<class RRR::YYY<class RRR::WWW<char,struct RRR::XXXs<char>,class RRR::UUU<char> >,class TTT::VVV *,struct RRR::less<class RRR::WWW<char,struct sRRR::XXX<char>,class RRR::UUU<char> > >,class RRR::UUU<struct RRR::pair<class std::WWW<char,struct RRR::XXX<char>,class RRR::UUU<char> > const ,class TTT::VVV *> >,0> >::ZZZ::meth1(void)"