Skip to content

Commit

Permalink
fix(Color): make constructor idempotent
Browse files Browse the repository at this point in the history
Problem:  passing a `Color` instance to the main constructor of `Color`
          (i.e. `Color.new()` or `Color()`) silently causes a bug and
          does not throw an error

Solution: immediately return the argument back to the caller when passed
          a `Color` instance, thereby making the constructor idempotent
  • Loading branch information
tmillr committed Jul 26, 2024
1 parent 8a311d4 commit 1a3f3b7
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 0 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,16 @@ The format is based on [Keep a Changelog], and this project adheres to [Semantic

## [unreleased]

### What's New?

- Added static/class function `Color.is_Color()` for detecting Color instances

### Changes

### Issues Fix

- Made `Color()` constructor idempotent (previously, passing a `Color` inst silently caused a bug)

## [v1.1.0] - 23 July 2024

### Highlight Groups
Expand Down
4 changes: 4 additions & 0 deletions lua/github-theme/lib/color.lua
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ local M = setmetatable(Color --[[@as GhTheme.Color.Static]], {
---@return GhTheme.Color
---@nodiscard
function M.new(color)
if M.is_Color(color) then
return color --[[@as GhTheme.Color]]
end

local ty = type(color)

if ty == 'string' or ty == 'number' then
Expand Down
29 changes: 29 additions & 0 deletions test/github-theme/color_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ describe('Color', function()
assert.are.same(ex.str, c:to_css())
end)

it('should construct from an integer', function()
local n = 0x12345630
local c = Color.from_hex(n)
assert.are.same(n, c:to_hex(true))
assert.are.same('#12345630', c:to_css(true))
end)

it('should error when given invalid args', function()
assert.does.match.error(function()
---@diagnostic disable-next-line: param-type-mismatch
Expand Down Expand Up @@ -99,6 +106,13 @@ describe('Color', function()
assert.are.same(ex.str, c:to_css())
end)

it('should construct from an integer', function()
local n = 0x12345630
local c = Color(n)
assert.are.same(n, c:to_hex(true))
assert.are.same('#12345630', c:to_css(true))
end)

it('should infer from rgba components', function()
local c = Color(Color.from_hex(ex.str):to_rgba())
assert.are.same(ex.hex, c:to_hex())
Expand All @@ -116,6 +130,21 @@ describe('Color', function()
-- assert.are.same(ex.hex, c:to_hex())
assert.are.same(ex.str, c:to_css())
end)

it('should be idempotent', function()
local orig = Color(ex.rgba)
orig.alpha = 0.5
local new = Color(orig --[[@as any]])

for _, c in ipairs({ orig, new }) do
for _, k in ipairs({ 'WHITE', 'BLACK', 'BG' }) do
rawset(c, k, c[k])
end
end

assert.are.same(orig, new)
assert.are.same(orig:to_css(true), new:to_css(true))
end)
end)

describe('conversion', function()
Expand Down

0 comments on commit 1a3f3b7

Please sign in to comment.