Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

16colors2shades #601

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

16colors2shades #601

wants to merge 3 commits into from

Conversation

sonjiku
Copy link

@sonjiku sonjiku commented May 3, 2021

  • I changed the generic_adjust function, so it produces 16 colors by default.
  • Added command line option "--nine". which reverts to the original 9 color output.
  • Colors 1-7 are darker shades of the latter 8.
  • All backend scripts, use the generic_adjust function, instead of "manualy adjusting". This means that the different backends, produce similar outputs, but not with the same palletes.

… colors.py, that makes pywal generate darker shades for 1-7
@sonjiku
Copy link
Author

sonjiku commented May 3, 2021

By the way, If someone wants the original functionality, one only needs to change his templates. Could perhaps be easier if you could choose this fith a command option.

@eylles
Copy link

eylles commented May 11, 2021

tried this version, if you run wal with --backend wal it produces the original functionality of 9 different colors

@sonjiku
Copy link
Author

sonjiku commented May 19, 2021

tried this version, if you run wal with --backend wal it produces the original functionality of 9 different colors

New commit should fix the issue.

@eylles
Copy link

eylles commented May 20, 2021

really good pr, hopefully it gets merged soon

@sonjiku
Copy link
Author

sonjiku commented May 20, 2021

By the way, If someone wants the original functionality, one only needs to change his templates. Could perhaps be easier if you could choose this fith a command option.

Now you can! ^^

@rarick
Copy link

rarick commented Jul 1, 2021

Also adding my support for this PR, hope it gets pulled in soon.

@eylles
Copy link

eylles commented Aug 27, 2021

@dylanaraps

@ZerdoX-x
Copy link

ZerdoX-x commented Nov 2, 2021

I really like it. Thank you! I guess I will stick to this fork for now.
With this feature building themes with hover/focus/active state will be easier for me
image

Wonder why isn't this merged yet?

@eylles
Copy link

eylles commented Nov 2, 2021

Perhaps making this a permanent fork ?

@eylles
Copy link

eylles commented Nov 2, 2021

@ZerdoX-x nice kaga btw
Screenshot at 2021-11-01 21-03-37

@eylles
Copy link

eylles commented Dec 14, 2021

going through the diff i think i see some reasons why this isn't getting merged any time soon.

  • change of default behaviour, if anything the default of 9 colors should be kept and the generation of 16 colors should be the new flag
  • 2 alacritty templates added, the templates should be their own PR
  • colorscheme paths now separated into 9 and 16 colors, if anything pywal should not make that difference and differentiating the number of colors in a theme should not be done in the path but rather appended to the theme name, as this breaks compatibility with the built in colorschemes like the base 16 ones
  • color 0 was changed shade for no good reason, the shade generated by default was perfectly fine

@dylanaraps did i hit the nail with this?

honestly i don't feel like doing this myself and opening a new PR just for that, nor forking for that matter, but if i have i will do it and create an additional branch to track upstream commits if 16 colors functionality is to never be merged.

@sonjiku
Copy link
Author

sonjiku commented Jan 6, 2022

Hey @eylles,

You have some valid points.

change of default behaviour, if anything the default of 9 colors should be kept and the generation of 16 colors should be the new flag

I agree with you, that 9 colors should most probably have been kept as the default functionality, as to change the familiarity of the colorschemes the software created by default up to now. Perhaps it is one of the reasons the pull request hasn't been merged, as if @dylanaraps wanted pywal to output 16 colors by default, they would have done so from the start.

The main reason I changed it, is because it seemed to me as if 16 colors was the sensible default.

2 alacritty templates added, the templates should be their own PR

I don't understand this. Adding the 2 extra templates, doesn't make sense to me as being a reason to not merge the pr. Why create another pull request when this is something extra added with this one?

colorscheme paths now separated into 9 and 16 colors, if anything pywal should not make that difference and differentiating the number of colors in a theme should not be done in the path but rather appended to the theme name, as this breaks compatibility with the built in colorschemes like the base 16 ones

I don't really understand what your suggested alternative here is.

color 0 was changed shade for no good reason, the shade generated by default was perfectly fine

I have no good reason for this. I guess just ocd. I just thought the code looked better that way.

I'm planning on returning to this pr and perhaps creating a permanent fork, as I don't see @dylanaraps ever mergin this. I'm just very busy with school atm.

@eylles
Copy link

eylles commented Jan 6, 2022

I like to put my actions where my mouth is, so i already started working on a branch to correct the probable reasons as to why the 16 colors functionality isn't being merged:
https://github.com/eylles/pywal/tree/16_cols

My blockade now is thinking of a good nane for the 16 cols flag, ideally this would be merged into pywal after the streamlined PR is done as i really do not want to maintain a fork of pywal since i really don't think it is ideal, bit if it has to come about that.... welp...

On the templates thing, the usual way i think of PRs is that they provide 1 feature or fix 1 bug, as such making pywal generate 16 color palettes would be 1 feature, while providing alacritty templates would be another 1 feature, thus being 2 features in the same PR...

@eylles
Copy link

eylles commented Feb 20, 2022

after quite some hiatus i finally got my ass to address the remaining work, also had to make the fast_colorthief backend use the adjust function.

https://github.com/eylles/pywal/tree/16_cols

@sonjiku i can always PR to your master branch first if u want.

@eylles eylles mentioned this pull request Feb 20, 2022
@eylles
Copy link

eylles commented Feb 28, 2022

yeh, forgot to mention now a permanent fork exists https://github.com/eylles/pywal16

already merged some other pending PRs.

@eylles
Copy link

eylles commented Feb 24, 2024

on the way to 3 years since and finally available at pypi https://pypi.org/project/pywal16/

@sonjiku
Copy link
Author

sonjiku commented Feb 24, 2024

@eylles I can see! Some time ago I found out that your fork of my fork made it's way to the AUR, has had videos made about it, and has even inspired some functionality in wallust, which is what I'm using right now. Until I get back to finishing a small project of mine, at least, haha.

I'm really happy with how a 2 days' summer project ended up being liked by as many people as it has! I'd appreciate though if some credit was given to me in your repo, since, even though you're the maintainer doing the heavy lifting, a lot of people think that you came up with this which is wrong.

Anyways, I appreciate you taking the torch and keeping it alive!

@eylles
Copy link

eylles commented Feb 24, 2024

I'm really happy with how a 2 days' summer project ended up being liked by as many people as it has! I'd appreciate though if some credit was given to me in your repo, since, even though you're the maintainer doing the heavy lifting, a lot of people think that you came up with this which is wron

i don't even credit myself on pywal16, author email still points to dylan and didn't even add a maintainer field, the commit history is king

image

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants