search

dascandy / evoke

Magic build tool
Apache License 2.0
166 stars 17 forks source link

clear mess left by reporter #89

Closed wrigleyster closed 2 years ago

dascandy commented 2 years ago

Thanks!

dascandy commented 2 years ago

Returning to this code and migrating it to the reporter in question, I'm having a hard time reproducing (or unit testing) the mess. Do you have a way for me to know if I reintroduced this issue?

wrigleyster commented 2 years ago

Unit testing it, I think would be a considerable headache, as it can only really be verified visually, if you really wanted to unit test it you'd have to count characters, and verify that the number of printed characters printed before, does not exceed the length of the error message.

Sadly, I don't have the code snippet that caused the issue, but I can explain in detail what the issue was.. with a bit of luck that will give you an idea of how to verify that the issue is not reintroduced.

The issue was with the ConsoleReporter. https://github.com/dascandy/evoke/blob/0f347da4f0723a452347ded37a3d267670523cfd/reporter/src/ConsoleReporter.cpp#L89 specifically \033[A which moves the cursor up to the line above, causing whatever is printed next, to be printed on the same line, effectively "overwriting" what was there before. The issue was that when the flow was interrupted by an error, printing an error message that was shorter than whatever was on the line before, the error message and the previous content([filename1][filename2][etc]...) were garbled together.

The \e[K I introduced solved the issue, because it clears the supposed-to-be empty space to the right of the error message, when printing it.

I'm guessing you are talking about the fuzzing branch? The simple fix would be to just leave the \e[K in. Basically you want it on any printed line that might interrupt the redraw. And if you have it in one place too many, it is harmless. Also, you don't need the #if defined introduced in #92 as the ConsoleReporter already assumes VT ANSI escape codes.

wrigleyster commented 2 years ago

Seems I don't need a code snippet of my own: Doing a quick compile and run of evoke from the fuzzing branch on my machine demonstrates the error immediately, I tried running evoke from within each of the fw/ and project/ folders. Seeing as I don't have all the headers installed locally, evoke complains that they are missing, spouting error messages garbled with [filename1][filename2][etc]...

To reproduce, I think you could probably introduce typos in a few of the filename headers, and watch as evoke fails to find them. You might need to introduce a few typos across a number of files to reproduce the issue reliably, as there is undoubtedly going to be a race condition between which output stream is flushed first, stdout or stderr. Eg. when I run it, it varies both which lines and how many lines are garbled.