Skip to content
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

tokyo-night theme uses tokyo-storm palette #702

Closed
adrian5 opened this issue Dec 19, 2023 · 7 comments · Fixed by #710
Closed

tokyo-night theme uses tokyo-storm palette #702

adrian5 opened this issue Dec 19, 2023 · 7 comments · Fixed by #710

Comments

@adrian5
Copy link
Contributor

adrian5 commented Dec 19, 2023

With the exception of the background, the colors of tokyo-night are a copy of tokyo-storm.

It would be nice to have the higher-contrast moon and night variants. You can see the original Tokyo Night theme variants here. I've looked around in this repository, but it's not apparent to me how to contribute themes (I assume there's some build step), so here a request instead.

Thanks!

@adrian5 adrian5 changed the title **tokyo-night** theme uses **tokyo-storm** palette tokyo-night theme uses tokyo-storm palette Dec 19, 2023
@fdncred
Copy link
Collaborator

fdncred commented Dec 19, 2023

We'd accept a PR for this.

@adrian5
Copy link
Contributor Author

adrian5 commented Dec 20, 2023

I would if I knew how. It seems the make.nu script is hardcoded for a specific remote repository and fails if I point it to my fork. Since it overwrites any local changes with a cloned repo, that won't work either.

@amtoine
Copy link
Member

amtoine commented Dec 20, 2023

@adrian5
why would it fail with a fork?

also, that's how i wrote the script in the first place to avoid having to write a whole bunch of themes myself, but if there is a better solution somewhere, it could be nice 😋

@adrian5
Copy link
Contributor Author

adrian5 commented Dec 21, 2023

@adrian5 why would it fail with a fork?

My bad, it fails when pointing SOURCE { remote: … } to a non-default branch. I moved the changes in my fork to master and that worked.

also, that's how i wrote the script in the first place to avoid having to write a whole bunch of themes myself, but if there is a better solution somewhere, it could be nice 😋

Fair enough. I think one hindrance to contributions is that this hinges on the lemnos repo staying maintained, which I'm not sure it is.


Anyway, as mentioned I was now able to build the nushell theme file. Can I just add it to themes/nu-themes as a PR and someone else commits a screenshot afterwards?

@amtoine
Copy link
Member

amtoine commented Dec 21, 2023

My bad, it fails when pointing SOURCE { remote: … } to a non-default branch. I moved the changes in my fork to master and that worked.

ooh i see 👌

Fair enough. I think one hindrance to contributions is that this hinges on the lemnos repo staying maintained, which I'm not sure it is.

also more than fair 😉

Anyway, as mentioned I was now able to build the nushell theme file. Can I just add it to themes/nu-themes as a PR and someone else commits a screenshot afterwards?

there might be a script for the screenshots, not sure
@fdncred probably knows 😋

@fdncred
Copy link
Collaborator

fdncred commented Dec 21, 2023

we can have a pr without a screenshot. yes, there are scripts to generate screenshots. i ran them on windows.

@adrian5
Copy link
Contributor Author

adrian5 commented Dec 21, 2023

Alright! I saw the scripts (the screenshot link points there) but currently don't have a Windows installation available. Will do a PR and then close this.

fdncred pushed a commit that referenced this issue Dec 22, 2023
Closes #702. I was wrong in thinking the "night" and "storm" variants
should be much different — they're not. It's the divergent "moon"
variant between them that was missing. Added.

I have slight doubts about the darker gray `#828bb8` being used for e.g.
`ls` columns, but I think that basically correct. The high-contrast
foreground color `#c8d3f5` is the bright variant of that and should show
up in most other contexts I reckon.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants