thefryscorer / schemer2

Terminal Colorscheme Generator and Converter
243 stars 9 forks source link

Workaround for 16 color Xresources #2

Closed guglicap closed 8 years ago

guglicap commented 8 years ago

in case in the Xres file there are less than 16 colors. Needed it myself.

thefryscorer commented 8 years ago

Hi,

The commit you sent me doesn't compile but I see you've fixed that in your most recent commit. Still, I can't merge this one.

I also sort of disagree with this method of repeating them. Terminals tend to have the first 8 colors as primary colors and the second 8 colors as secondary colors. So doubling them up like this will cause the primary colors to be pairs of the first 4. That is, the first two primary colors would be the same, the second two primary colors would be the same.

Really, in the case that there's only 8 colors, it should repeat those 8 as the secondary colors. If there's only 4 colors, it should repeat them as a sequence of 4. Etc.

This is really more of an issue with the assumption that having less than 16 colors scraped from the config is an error condition. I will change the part that handles that case to just repeat the sequence of how ever many colors there were, instead of aborting.

guglicap commented 8 years ago

Hello, Thank you for your reply I understand why you won't merge, and I would like to implement what you describe, if it's okay to you.

On Aug 22, 2016 00:56, "Daniel Byron" notifications@github.com wrote:

Hi,

The commit you sent me doesn't compile but I see you've fixed that in your most recent commit. Still, I can't merge this one.

I also sort of disagree with this method of repeating them. Terminals tend to have the first 8 colors as primary colors and the second 8 colors as secondary colors. So doubling them up like this will cause the primary colors to be pairs of the first 4. That is, the first two primary colors would be the same, the second two primary colors would be the same.

Really, in the case that there's only 8 colors, it should repeat those 8 as the secondary colors. If there's only 4 colors, it should repeat them as a sequence of 4. Etc.

This is really more of an issue with the assumption that having less than 16 colors scraped from the config is an error condition. I will change the part that handles that case to just repeat the sequence of how ever many colors there were, instead of aborting.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/thefryscorer/schemer2/pull/2#issuecomment-241287628, or mute the thread https://github.com/notifications/unsubscribe-auth/AQdWWF3bkKESsCHeJkRG1gOcrpe7uwPlks5qiNe6gaJpZM4JpWzB .

thefryscorer commented 8 years ago

Hi again,

I've already made the change that I described before your reply. You can check out the latest commit, and verify that it solves the problem you had.

I added a few test cases for less than 16 color xterm configs, but I'm not entirely sure if the tests are indicative of real world use because I don't personally use Xresources.

guglicap commented 8 years ago

Okay, I see. Keep up the good work :) I'm closing this pull request.