Fix console mode

[Forgind]

Fixes #9079

Context

First stab at taking into account console mode when writing error messages as part of a user asking for a particular property/item/target result. If we can't verify that the console is VT-compatible, we fall back to just printing things out with their default coloration. (Though not perfect, I think this is an unimportant enough scenario, that I don't think it's worth worrying about.)

Changes Made

Testing

Notes

[Forgind]

Since the other PR is still open, do you want to fix this problem in there?

My personal preference is to fix it in a separate (i.e., this) PR, mostly because if I move these changes to that PR, it'd need to be reviewed more and probably need at least one or two more changes, and I'd rather it go in 🙂

[Forgind]

I can move it over there if you want me to, though.

[JanKrivanek]

I have couple minor comments for consideration.

The main things I'd like to see addressed:

• Prevent merge mess, by either basing this on your other branch, or by first waiting for that other PR to go in (and then resolving here)
• I believe the same issue should be addressed in other affected loggers as well (or otherwise the Inspect ConsoleMode before sending control codes on Windows #9079 should be left open)

[Forgind]

Member

I have couple minor comments for consideration.

The main things I'd like to see addressed:

• Prevent merge mess, by either basing this on your other branch, or by first waiting for that other PR to go in (and then resolving here)

100% agree. My plan is to wait until the other PR is in, then restart the merge, and I believe it will drop all the prior commits (and make your life a lot easier!)

• I believe the same issue should be addressed in other affected loggers as well (or otherwise the Inspect ConsoleMode before sending control codes on Windows #9079 should be left open)

Which loggers are you thinking of? The only loggers that I think are potentially susceptible to this sort of problem are this new logger (fixed in this PR), the terminal logger (fixed in one of my previous PRs), and the console logger, but I asked about fixing the console logger in #9079, and rainersigwald suggested it doesn't need fixing. I'm fine with trying to fix it or not fix it as the two of you agree 🙂

[JanKrivanek]

• I believe the same issue should be addressed in other affected loggers as well (or otherwise the Inspect ConsoleMode before sending control codes on Windows #9079 should be left open)

Which loggers are you thinking of? The only loggers that I think are potentially susceptible to this sort of problem are this new logger (fixed in this PR), the terminal logger (fixed in one of my previous PRs), and the console logger, but I asked about fixing the console logger in #9079, and rainersigwald suggested it doesn't need fixing. I'm fine with trying to fix it or not fix it as the two of you agree 🙂

Those are the loggers I had in mind. If all are already addressed separately - than we're good

[JanKrivanek]

Looks good (just marking it as not subject for merging so that it doesn't get merged by anybody else prior the other PR is in and conflicts resolved)

Please reconsider what win32 api details needs to be exposed beyond boundaries of the NativeMethods

[Forgind]

Please reconsider what win32 api details needs to be exposed beyond boundaries of the NativeMethods

That's fair. I guess I was thinking about if a user happens to be familiar with the wind32 API already...but how many people actually are? I added an enum.

[Forgind]

@JanKrivanek, diff looks a lot cleaner now, so I removed the do-not-merge label.

[JanKrivanek]

@JanKrivanek, diff looks a lot cleaner now, so I removed the do-not-merge label.

Thanks for all the adjustments! Looks good to go to me!

[rokonec]

LGTM. I have few opinionated/nit comments.

[Forgind]

@rokonec, I resolved your first and third comments but left your second one because you fixed that in your PR. As I said there, we can merge one of the PRs and resolve conflicts then, or I can pull your changes into my branch, and we can merge them as one change. I'm ok with either way.