Skip to content

Conversation

@dgw
Copy link
Member

@dgw dgw commented Jun 28, 2021

Description

Following up on an old TODO comment, now possible because we no longer support Python 2. Making this an Enum means it's safe against plugins accidentally (or intentionally) overwriting the color values.

This isn't the appropriate time to address such an issue, but the color name 'TEAL' doesn't exist in mIRC's documentation, nor is it listed at the "horse docs" I also don't know why 'OLIVE' is an alias to 'ORANGE'.

No color names have been changed yet, but I did add comments to point out the issue during future improvements.

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make quality and make test)
  • I have tested the functionality of the things this change touches

Notes

Compatibility

If a plugin is using the 7.x formatting.colors class as documented and intended, no problem. Any code pulling funny business like overriding color names for convenience will have problems, but we're not going to hand-hold people who went s(o)pelunking in the plugin API's innards and messed with stuff. Their code will break, and they'll have to fix it.

Future plans

Stuff like 'TEAL' and 'OLIVE' can be deprecated by creating a custom EnumMeta subclass that (in our case) would invoke the tools.deprecated machinery similar to how the ValidatedAttribute with parse=bool stuff works in 7.1. However this PR is just to take care of the immediate, easy conversion from a boring, basic class to an Enum. Fancy stuff later!

Following up on an old TODO comment, now possible because we no longer
support Python 2. Making this an Enum means it's safe against plugins
accidentally (or intentionally) overwriting the color values.

This isn't the appropriate time to address such an issue, but the color
name 'TEAL' doesn't exist in mIRC's documentation, nor is it listed at
the "horse docs": https://modern.ircdocs.horse/formatting.html#colors
I also don't know why 'OLIVE' is an alias to 'ORANGE'.

No color names have been changed *yet*, but I did add comments to point
out the issue during future improvements.
@dgw dgw added this to the 8.0.0 milestone Jun 28, 2021
@dgw dgw requested a review from a team June 28, 2021 03:19
@dgw dgw merged commit d2014f9 into master Jul 5, 2021
@dgw dgw deleted the formatting.colors-enum branch July 5, 2021 06:02
@dgw dgw added the Breaking Change Stuff that probably should be mentioned in a migration guide label Jul 5, 2021
@dgw dgw mentioned this pull request Nov 11, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking Change Stuff that probably should be mentioned in a migration guide Low Priority Tweak

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants