Skip to content

Remove need for leading # on colour codes#28

Merged
jackdevey merged 2 commits into
mainfrom
remove-#-requirements
Apr 16, 2022
Merged

Remove need for leading # on colour codes#28
jackdevey merged 2 commits into
mainfrom
remove-#-requirements

Conversation

@jackdevey

@jackdevey jackdevey commented Apr 16, 2022

Copy link
Copy Markdown
Owner
  • Validates hexadecimal colour codes using Regex
  • Remove all instances of leading #
  • Fixes 27

@jackdevey
Remove need for leading # on colour codes
e979c0f 
 - Validates hexadecimal colour codes using Regex

 - Remove all instances of leading #

 - Fixes 27
@jackdevey jackdevey linked an issue Apr 16, 2022 that may be closed by this pull request
lux color command entered as show in example gives error "Not enough arguments required" #27
Closed
@jackdevey

jackdevey commented Apr 16, 2022

Copy link
Copy Markdown
Owner Author

Hi Russell

With these changes the leading # is optional in colour codes

In Command Prompt

The following is valid syntax:

lux color 0 #0067f4

lux color 0 0067f4

In Powershell

The following is valid syntax:

lux color 0 `#0067f4

lux color 0 "#0067f4"

Any other way to indicate the colour code isn't a string

lux color 0 0067f4

Also

  • For simplicity all instructions should probably not include the leading # as it is the only method that works everywhere. I have done this in all error messages.
  • Colour codes are validated using a regular expression which is better too now as it is a much more rigorous validation than we had before.

NOW PLEASE ACCEPT MY MERGE :) 🐫🐫

@jackdevey jackdevey marked this pull request as ready for review April 16, 2022 18:32
@jackdevey jackdevey requested a review from russellbanks April 16, 2022 18:32
@jackdevey jackdevey added the bug Something isn't working label Apr 16, 2022
russellbanks
russellbanks approved these changes Apr 16, 2022
View reviewed changes

@russellbanks russellbanks left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jackdevey jackdevey merged commit 91bc78e into main Apr 16, 2022
@jackdevey jackdevey deleted the remove-#-requirements branch April 16, 2022 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@russellbanks russellbanks russellbanks approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

lux color command entered as show in example gives error "Not enough arguments required"

2 participants

@jackdevey @russellbanks