obsproject / obs-bot

Source code for the obsproject Discord bot
https://discord.gg/obsproject
GNU General Public License v3.0
38 stars 18 forks source link

runner: Add coloured logging #2

Open WizardCM opened 3 years ago

WizardCM commented 3 years ago

image

After a few minutes of using the bot, I realised it'd benefit from some basic colour coding for logging levels.

derrod commented 3 years ago

Few things:

  1. For consistency with the rest of the code, use single-quoted strings
  2. printf style string formatting should not be used, use the modern formatting methods (.format/f-strings) instead
  3. There really isn't any reason for formatting at all, the level names don't change, so you can just write out the level name + console colour codes for each, also saves the unnecessary string concatenation with the RESET part
  4. The way this is currently done would affect both the console and file log handlers, the logfile should not contain terminal control characters and remain plain text, you may have to introduce a separate log formatter for console logging
WizardCM commented 3 years ago

4) did cross my mind as I went to bed. Definitely worth fixing. I'll get all 4 points fixed this week.

WizardCM commented 3 years ago

Alright, I removed formatting entirely & ensured it doesn't affect log files. Let me know if it needs anything else.

derrod commented 3 years ago

Well this is a bit of an issue. screenshot I wonder if there's a cross-platform way to do this.

WizardCM commented 3 years ago

Hmm, this is a good point. I don't feel particularly comfortable importing a dependency just to colour logs. I'll do some further digging. Do we want to support cmd on Windows 10 as the minimum?

derrod commented 3 years ago

Yeah, somewhat selfishly since I do most of my work on windows, so it should work there. Haven't tested in powershell, but no sane person uses powershell.

tt2468 commented 3 years ago

I don't feel particularly comfortable importing a dependency just to colour logs.

The bot is already using third party requirements (requirements.txt), so I don't see why it would be a bad thing. I personally believe that it's better to depend a package that results in more readable code than it is to try to build in that functionality myself in situations like this.