On Tue, Sep 11, 2018 at 9:51 PM, Henri Verbeet hverbeet@gmail.com wrote:
On 11 September 2018 at 23:05, Huw Davies huw@codeweavers.com wrote:
It makes following the flow of the code incredibly difficult. If you don't believe me, come back to this patch in a month and see how you feel then.
Yeah, this. You have goto's in fairly deeply branched code, inside a nested switch, jumping to a branch somewhere inside the outer switch, all of which happens inside a relatively long function.
I see. I usually view switch cases as similar to labels and don't mind goto between nested switch if it jumps near the actual case of the switch and not deep within it (it's basically like sharing the same "case" in my mind), but point taken. I admit that mindset applies to small switch/case blocks though, not like this function.
To be honest, I think it will be easier if I skip the no-op patch because this will allow me to allocate the hwndText in the helper function itself and skip passing it as argument, and encapsulate it there along with other stuff like the bools. Of course, most of the function will still be a cut & paste just like before (after the realloc part) so it will only seem artificially large but not that many changes (the control_char handling can be simplified by checking all of them for len of zero, which is better anyway). There's no issue with this, I hope? Or do I really have to make the no-op patch first?