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

add a collection of colorschemes for nushell #286

Merged
merged 11 commits into from
Sep 7, 2022

Conversation

amtoine
Copy link
Member

@amtoine amtoine commented Aug 29, 2022

This PR addesses #285.

Current implementation

  1. the themes from lemnos/theme.sh have been added as a submodule in ./themes/lemnos/ to easily update them as the source repo changes
  2. a template theme file has been added as template.nu -> it is just a rewrite of the default dark theme with the standard "colorxx" names in replacement
  3. a make.nu file which converts the files from ./themes/lemnos/themes/ to usable nushell modules in ./themes/themes/
  4. the themes have been generated
  5. some instructions in the README

Open discussions

  1. any usual PR stuff 😋
  2. does the template lead to great nushell themes? i've tried the commands in the README a few times but it's hard to tell what could be improved in the template for better themes 🤔
  3. is the submodule a great idea?

To test the themes

Please run

use ./themes/themes/THEME.nu
let-env config = ($env.config | merge {{color_config: (THEME)}})

remove the useless colors.nu script
the `map.csv` file:
```csv
code,bold,bash,nushell
0,false,black,black
1,false,red,red
2,false,green,green
3,false,yellow,yellow
4,false,blue,blue
5,false,magenta,purple
6,false,cyan,cyan
7,false,white,light_gray
8,true,black,dark_gray
9,true,red,red_bold
10,true,green,green_bold
11,true,yellow,yellow_bold
12,true,blue,blue_bold
13,true,magenta,purple_bold
14,true,cyan,light_cyan
14,true,cyan,cyan_bold
15,true,white,white
```

the command used was
```nushell
open map.csv |
each {
    |it|
    sed -i $"s/: ($it.nushell)$/: \"{{color($it.code)}}\"/" template.nu
}
```
or as a oneliner
```nushell
open map.csv | each {|it| sed -i $"s/: ($it.nushell)$/: \"{{color($it.code)}}\"/" template.nu}
```
@fdncred
Copy link
Collaborator

fdncred commented Aug 29, 2022

Nice! Here are some thoughts.

  1. I don't really want to have a sub-module. We can leave it for now, convert everything, then just delete it.
  2. I really want to have these items in there but commented out so we'll know what the desired background/foreground/cursor should be per theme. maybe i just missed it?
background #090300
foreground #a5a2a2
cursor #a5a2a2
  1. other than that, it's really close to what i was thinking. and i like your example about how to use a theme.

@amtoine
Copy link
Member Author

amtoine commented Aug 29, 2022

  1. I don't really want to have a sub-module. We can leave it for now, convert everything, then just delete it.

the submodule can be deleted afterwards, definitely
we'll just have to check new themes on the lemnos side then maybe, but not a heavy burden in the end 👍

  1. I really want to have these items in there but commented out so we'll know what the desired background/foreground/cursor should be per theme. maybe i just missed it?

nope, that's my fault
i'll add these shortly!

  1. other than that, it's really close to what i was thinking. and i like your example about how to use a theme.

that's coool 😋

glad you like the PR 😌
i do not have the time now, but i'll add the rest shortly 💪

@fdncred fdncred marked this pull request as draft August 29, 2022 20:31
@fdncred
Copy link
Collaborator

fdncred commented Aug 29, 2022

i made it a draft until we're ready to land.

This commit adds the background, the foreground and the cursor
colors to the themes as commented lines.
These do not fit into any nushell theme as they should be set by the
terminal emulator, but they are included here for completeness.

Addresses nushell#286 (comment)
@amtoine
Copy link
Member Author

amtoine commented Sep 4, 2022

@fdncred

hello there 👋

i've changed the template.nu file to include the extra desired colors in ee65e18 👍

remaining caveats

  1. the submodule is still present but will be removed at the end of the PR, before merging as we agreed
  2. there is an error at the end of ./make.nu which i do not understand well
> ./make.nu
3024
3024-day
...
yachiyo
zenburn
Error: nu::shell::name_not_found (link)

  × Name not found
    ╭─[./make.nu:18:1]
 18 │             |it|
 19 │             sed -i $"s/{{($it.name)}}/($it.rgb)/g" $theme_file
    ·                                            ─┬─
    ·                                             ╰── did you mean 'name'?
 20}
    ╰────

but the zenburn theme is created as well without any problem 🤔
3. should we ship the themes themselves or juste the README.md, the template.nu and the make.nu files and let the user run ./make.nu and generate the themes? 🤔

@fdncred
Copy link
Collaborator

fdncred commented Sep 4, 2022

Looking good but we need to remove sed and just use nushell code too.

@amtoine
Copy link
Member Author

amtoine commented Sep 7, 2022

cool, i'll work on that then 😋

EDIT: that should be great in 9c38696 👌

@fdncred
Copy link
Collaborator

fdncred commented Sep 7, 2022

ok, nice! it looks good. now we just have to remove the sub module and you already have the attribution. so this is looking very close to me.

@fdncred
Copy link
Collaborator

fdncred commented Sep 7, 2022

nice work @amtoine. let's land this. appreciate you working with me. ❤️

@fdncred fdncred marked this pull request as ready for review September 7, 2022 18:48
@fdncred fdncred merged commit bdfed70 into nushell:main Sep 7, 2022
@WindSoilder
Copy link
Contributor

@amtoine Thank you, I really love this. Just select one theme for my usage :-D

@amtoine amtoine deleted the pr/feature/lemnos-themes branch September 8, 2022 08:03
@amtoine
Copy link
Member Author

amtoine commented Sep 8, 2022

@fdncred

nice work @amtoine. let's land this. appreciate you working with me. heart

oooh these are nice words 😮
shared 😌 😉

@WindSoilder

@amtoine Thank you, I really love this. Just select one theme for my usage :-D

i'm glad it works and is usefull 🤩

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 this pull request may close these issues.

None yet

3 participants