X colors #239

denismaxim0v · 2021-03-26T14:44:10Z

that's still a wip

… the palette

matu3ba · 2021-04-23T14:08:51Z

You can mark it as WIP: or DRAFT: to prevent CI running.

…re customizable colored elements

imsnif · 2021-04-26T09:20:52Z

src/common/utils/shared.rs

+}
+
+pub fn load_palette() -> Palette {
+    let palette = match Colors::new("xresources") {


@denismaxim0v - my guess is that if you move this call to OsApi and then implement something that returns the defaults for the test fixtures, then the tests should pass.

Let me know if you want more detailed instructions :)

Well, that makes sense, I will have a go at it this week hopefully

TheLostLambda · 2021-04-27T13:37:32Z

By the way @denismaxim0v , how have you felt about the ModeInfo solution? Would it be better to just have a function like get_palette() -> Palette? That way you wouldn't need to write anything to handle it in update(). I've decided to do that for something I should push later today and I think it might be a bit simpler.

In my mind a palette isn't really an Event I suppose, but I'd like to know what you think, as my thinking on this has changed just today.

denismaxim0v · 2021-04-27T14:55:02Z

By the way @denismaxim0v , how have you felt about the ModeInfo solution? Would it be better to just have a function like get_palette() -> Palette? That way you wouldn't need to write anything to handle it in update(). I've decided to do that for something I should push later today and I think it might be a bit simpler.

In my mind a palette isn't really an Event I suppose, but I'd like to know what you think, as my thinking on this has changed just today.

I think it could actually be an event if we have some predefined themes the user should be able to switch between them from some menu, like strider but for themes maybe?

TheLostLambda

This is awesome! Works like a charm on my machine and seems easy to use in plugins!

With that being said, I'd like to see if we could move a couple of things out of zellij-tile. I'm looking to keep it as a very minimalist wrapper library so that it's easy to port to other languages. If I understand correctly, I think all we need is Theme, PaletteSource, and Palette in zellij-tile.

We could then remove everything but the two enums and one struct.

If you would like to have these things in a library, maybe we could spin off a zellij-tile-extra or something that's meant to add helpful functionality for Rust plugins in particular. As long as the zellij-tile library itself is as small and simple as possible.

TheLostLambda · 2021-05-01T13:27:42Z

default-plugins/status-bar/src/main.rs

@@ -41,6 +32,153 @@ impl Display for LinePart {
    }
 }

+#[derive(Clone, Copy)]
+pub struct ColoredElements {
+    // slected mode


Looks like a typo of selected here :)

definitely, mb

TheLostLambda · 2021-05-01T13:30:47Z

default-plugins/status-bar/src/main.rs

+    pub superkey_suffix_separator: Style,
+}
+
+impl ColoredElements {


I'd personally not bother with the new() function, since the style fields are already public. I'd directly construct it as:

ColoredElements { selected_prefix_separator: // Something... // ... }

Instead of calling new() in a similar way :)

Yeah, that's fair

TheLostLambda · 2021-05-01T13:31:48Z

default-plugins/status-bar/src/main.rs

+// that can be defined in the config perhaps
+fn color_elements(palette: Palette) -> ColoredElements {
+    match palette.source {
+        PaletteSource::Default => ColoredElements::new(


Here is where I would just do this thing instead:

ColoredElements { selected_prefix_separator: // Something... // ... }

TheLostLambda · 2021-05-01T13:35:09Z

default-plugins/status-bar/src/main.rs

+    }
+}
+
+// I really hate this, but I can't come up with a good solution for this,


So the default and Xresources are actually themed differently? As opposed to just having a different palette?

That was only needed, because I wanted to keep the original default coloring, and as for the Xresources I wanted to make it slightly different, because if mapped as default it looked horrific and vice versa. Also I think that could be moved somewhere else or something similar to be done to color individual elements of Zellij into whatever you want(as in one of the discussions we started on discord), not sure if this is a cool feature or not tho

TheLostLambda · 2021-05-01T13:37:39Z

src/common/mod.rs

@@ -267,7 +267,8 @@ pub fn start(mut os_input: Box<dyn OsApi>, opts: CliArgs) {
            let send_plugin_instructions = send_plugin_instructions.clone();
            let send_app_instructions = send_app_instructions.clone();
            let max_panes = opts.max_panes;
-
+            let colors = os_input.load_palette();
+            // debug_log_to_file(format!("{:?}", colors));


Could probably clean this up :)

(The commented line, that is)

Ah yes, bad habits

TheLostLambda · 2021-05-01T13:40:16Z

zellij-tile/src/data.rs

+    Default,
+    Xresources,
+}
+pub mod colors {


Out of curiosity, why do we need this when we have palettes? Is this used within plugins?

This seems to be for the Default implementation? See the below comment :)

it seems that it got lucky and survived when I was cleaning stuff up

TheLostLambda · 2021-05-01T13:43:02Z

zellij-tile/src/data.rs

+    pub orange: (u8, u8, u8),
+}
+
+impl Default for Palette {


Can this be done on the Zellij side of things? It's fine to just #[derive(Default)] in zellij-tile because plugins are guaranteed to receive a ModeUpdate when they are loaded. I'd rather this default palette be made in Zellij as opposed to here if possible :)

TheLostLambda · 2021-05-01T13:43:47Z

zellij-tile/src/lib.rs

@@ -39,3 +39,19 @@ macro_rules! register_plugin {
        }
    };
 }
+


Would these macros be able to live in the status bar?

These are used in tab-bar too, and anywhere pretty much where you want to color stuff, those would be needed, so I figured maybe here is the best place for them?

I think that's fair, but I don't quite think they are essential to the functioning of the API (especially if we are going to have "invisible" plugins soon that don't need a Pane to run).

How would you feel about spinning out a zellij-tile-extra library or something like that in the main repo (just like zellij-tile) that can hold non-critical but potentially quite useful helper functions / macros? Like I said, I'm quite invested in keeping zellij-tile as slim and minimal as possible, but I'd be happy putting almost anything in a library like zellij-tile-extra. I think having that additional "utility" library could actually help us clean up the plugins we have so far (eventually) 🙂

…alette in zellij-tile

TheLostLambda · 2021-05-02T10:02:11Z

zellij-tile/Cargo.toml

@@ -7,6 +7,7 @@ description = "A small client-side library for writing Zellij plugins"
 license = "MIT"

 [dependencies]
+ansi_term = "0.12.1"


Can this be removed now that zellij-tile-extra exists?

You are very quick :), yes it can

TheLostLambda

Nice! Thanks for letting me bully you about when it came to zellij-tile! I think this is looking great and that you should merge it whenever you're ready! 🙂

denismaxim0v · 2021-05-02T10:08:18Z

Nice! Thanks for letting me bully you about when it came to zellij-tile! I think this is looking great and that you should merge it whenever you're ready!

Well, I'm pretty much ready. Maybe will wait a bit for others to have a look at it. And we are ready to post on r/unixporn 😄

denismaxim0v added 2 commits March 25, 2021 18:05

wip: reading colors from xrdb

a35cade

wip: scary how messy it is, but it's a start

2c402b0

denismaxim0v marked this pull request as draft March 26, 2021 14:44

denismaxim0v added 13 commits March 27, 2021 14:43

wip: latest plugin system merge in

b71315b

wip: move Palette out of zellij-tile

6e276ae

wip: replace the impl with a fn load_palette instead

42890d4

wip: need to set the default for Palette in the plugins

2b59edb

wip: does it take too long to load colors from xrdb?

b8ee02d

wip: sending the ModeInfo on initial plugin load

a6ad8e6

wip: need to figure out how to clear lines

7774edd

wip: cleaning up a bit, need to map the colors to the right places in…

748a7ff

… the palette

wip: looks cleaner

b9f3e35

Merge branch 'main' into x-colors

e5526df

wip: fixing the first arrow

b2139f4

wip: merge main in

8d98ca7

wip: new build?"

ae35379

denismaxim0v added 6 commits April 24, 2021 09:18

wip: merge main

47a918a

wip: the only way to make the default look like on main is to make mo…

fef466c

…re customizable colored elements

wip: colors adjustments

c44de89

wip: tab bar coloring

de72d3d

wip: trying to make it less messy so people don't hate me

2e94ef5

wip: I really don't want people to hate me

12388c9

denismaxim0v requested review from TheLostLambda, imsnif, a-kenji, h3nill and qballer April 24, 2021 15:40

wip: more cleaning up

a8adfdf

imsnif reviewed Apr 26, 2021

View reviewed changes

TheLostLambda mentioned this pull request Apr 27, 2021

Color scheme support #390

Closed

denismaxim0v added 4 commits May 1, 2021 13:45

chore: painless merge this time

eabecde

chore: cleanup, fix: the tests finally pass

f858229

fix: hello formatter my old friend...

0adc914

chore: clippy please allow it

61bb74e

denismaxim0v marked this pull request as ready for review May 1, 2021 12:21

denismaxim0v requested a review from imsnif May 1, 2021 12:21

TheLostLambda suggested changes May 1, 2021

View reviewed changes

denismaxim0v added 2 commits May 2, 2021 11:59

Merge branch 'main' into x-colors

452940f

fix: don't have new on ColoredElements anymore, removed default for p…

d48cc81

…alette in zellij-tile

denismaxim0v requested a review from TheLostLambda May 2, 2021 09:36

chore: added zellij-tile-extra, moved macros from zellij-tile

56cf446

TheLostLambda reviewed May 2, 2021

View reviewed changes

chore: remove unused dependency from zellij-tile

ce1f81b

TheLostLambda approved these changes May 2, 2021

View reviewed changes

denismaxim0v merged commit 98d9eac into main May 2, 2021

a-kenji added a commit that referenced this pull request May 2, 2021

docs(changelog): Add theme support through xrdb #239

ecd3efc

h3nill deleted the x-colors branch May 4, 2021 03:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

X colors #239

X colors #239

denismaxim0v commented Mar 26, 2021

matu3ba commented Apr 23, 2021

imsnif Apr 26, 2021

denismaxim0v Apr 27, 2021

TheLostLambda commented Apr 27, 2021

denismaxim0v commented Apr 27, 2021

TheLostLambda left a comment

TheLostLambda May 1, 2021

denismaxim0v May 1, 2021

TheLostLambda May 1, 2021

denismaxim0v May 1, 2021

TheLostLambda May 1, 2021

TheLostLambda May 1, 2021

denismaxim0v May 1, 2021

TheLostLambda May 1, 2021

TheLostLambda May 1, 2021

denismaxim0v May 1, 2021

TheLostLambda May 1, 2021

TheLostLambda May 1, 2021

denismaxim0v May 1, 2021

TheLostLambda May 1, 2021

TheLostLambda May 1, 2021

denismaxim0v May 1, 2021

TheLostLambda May 2, 2021

TheLostLambda May 2, 2021

denismaxim0v May 2, 2021

TheLostLambda left a comment

denismaxim0v commented May 2, 2021

@@ @@ -39,3 +39,19 @@ macro_rules! register_plugin { @@
                       }
                   };
               }

X colors #239

X colors #239

Conversation

denismaxim0v commented Mar 26, 2021

matu3ba commented Apr 23, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheLostLambda commented Apr 27, 2021

denismaxim0v commented Apr 27, 2021

TheLostLambda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheLostLambda left a comment

Choose a reason for hiding this comment

denismaxim0v commented May 2, 2021