-
Notifications
You must be signed in to change notification settings - Fork 189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added ability to change theme from command line arguments #177
Conversation
Nice, thanks! There's also #166 by @AshenDrops which instead allows adding colour schemes via separate files. We should consider which approach (or both?) is best to have. |
@@ -887,20 +894,42 @@ def main(): | |||
help='keybinding for alternate up key') | |||
key_group.add('--key-down', default='j', | |||
help='keybinding for alternate down key') | |||
|
|||
#add color scheme options | |||
col_group = parser.add_argument_group('Colors') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also add the existing --col-scheme
option to this group?
If you run the changed files against pep8, it will point out a few other suggestions (I should add that to the automatic check as well). |
I spotted #166 after I wrote most of the code but I decided that since this lets you override single values instead of the whole theme I would finish it anyway. It should just slot in next to JSON code with no issues as $166 loads more themes are start-up while this just lets you override values of the selected theme. I’ll have a look at the other issues now and see what I can do. |
…i/ so it passes the standard pep8 tests
Hopefully that should fix all the issues. Let me know if there is anything else. |
col_group.add('--col-scheme', choices=COL_SCHEMES.keys(), | ||
default='default', help='colour scheme to use') | ||
col_group.add('--col-palette-colors', choices=('16', '88', '265'), | ||
default=16, help='Amount of available colors') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last choice should be 256
instead of 265
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, yes it should be. Fixed now
Thanks again! |
Added ability to change theme from command line arguments
Adds the ability to theme hangups from the command line and config file.
learnt how to use lint. this will pass the tests, sorry about the pull spam.