[Feature Request] Pull colors from pywal #60

make-42 · 2024-07-11T23:37:14Z

This works...

make-42 · 2024-07-12T00:54:37Z

Closes #59

nikp123

There's a bunch of changes I'd like you to make, if you're not sure how to proceed, ask me for help or let me implement them myself on my own time.

nikp123 · 2024-07-12T13:07:55Z

src/xava.c

@@ -224,6 +224,22 @@ as of 0.4.0 all options are specified in config file, see in '/home/username/.co
        thing.xava = &xava;
        xavaIONotifyAddWatch(&thing);

+        // attach pywal colors file to the IONotify thing
+        static const char b[] = "/.cache/wal/colors";


Multiple things need to be fixed:

I'd like to comply with the XDG base specification, this means that we should NEVER point to a specific folder, but rather let that depend on the environment variables and if they don't exist only THEN grab the hardcoded value. Since this isn't completely the first time this is done, I'd like this to be done within the src/shared (potentially a new config function??) family of functions (I'll do these myself if you don't want to make them)

pywal support should be optional, the current implementation will fail if the user doesn't happen to use pywal. Consider users of Windows or MacOS, this will straight up fail for them

nikp123 · 2024-07-12T13:08:34Z

src/xava.c

+        int lenb = strlen(b);
+        char *filename = malloc(lena+lenb+1);
+        // copy & concat (including string termination)
+        memcpy(filename,getenv("HOME"),lena);


I know I may come out as not-C-like but I'd like to avoid doing pointer math as much as possible.

nikp123 · 2024-07-12T13:11:26Z

src/xava.c

+        memcpy(filename,getenv("HOME"),lena);
+        memcpy(filename+lena,b,lenb+1);   
+        struct xava_ionotify_watch_setup pywal;
+        pywal.xava_ionotify_func = &handle_ionotify_call;


Potential improvment: Since these changes are mostly colour related, we don't really need to reload the entire visualizer, instead we can just apply colours during runtime using a seperate handle_ionotify_call or make handle_ionotify_call handle logic differently based on the ID parameter (we can use it to find out what triggered the callback).

Something like: XAVA_IONOTIFY_CALLBACK_MAIN and XAVA_IONOTIFY_CALLBACK_PYWAL just to make it clear for other people reading the code.

nikp123 · 2024-07-12T13:12:30Z

src/xava.c

+        pywal.xava_ionotify_func = &handle_ionotify_call;
+        pywal.filename = filename;
+        pywal.ionotify = xava.ionotify;
+        pywal.id = 1;


This is the IONotify callback ID I'm talking about.

nikp123 · 2024-07-12T13:15:52Z

src/output/graphical_x11/main.c

@@ -172,6 +172,38 @@ void calculateColors(XAVA_CONFIG *conf) {
    }
    snatchColor("color5", conf->color, &conf->col, databaseName, &xavaXResDB);
    snatchColor("color4", conf->bcolor, &conf->bgcol, databaseName, &xavaXResDB);
+    int lineNumberFg = 1;


Lets avoid having color logic inside of the output methods themselves, but rather move that somewhere where it could be reused easily, like one of the shared functions. This way we avoid bloating the modules themselves unnecessarily.

src/shared/config/pywal sounds like a good place to start.

nikp123 · 2024-07-12T13:16:10Z

src/output/graphical_wayland/main.c

@@ -233,6 +233,39 @@ EXP_FUNC void xavaOutputLoadConfig(XAVA *hand) {
    XAVA_CONFIG *p = &hand->conf;
    xava_config_source config = hand->default_config.config;

+    int lineNumberFg = 1;


Same story applies here

nikp123 · 2024-07-12T13:16:40Z

src/output/graphical_x11/main.c

@@ -172,6 +172,38 @@ void calculateColors(XAVA_CONFIG *conf) {
    }
    snatchColor("color5", conf->color, &conf->col, databaseName, &xavaXResDB);
    snatchColor("color4", conf->bcolor, &conf->bgcol, databaseName, &xavaXResDB);
+    int lineNumberFg = 1;
+    int lineNumberBg = 2;
+    static const char b[] = "/.cache/wal/colors";


Same story when it comes to paths and pointer math

make-42 · 2024-07-12T13:39:32Z

I'll do my best!

nikp123 · 2024-07-12T13:40:25Z

I'll do my best!

Good luck and have fun

make-42 · 2024-07-12T14:39:23Z

I made all the changes you asked for except for the dynamic color reload part because I have no idea how to change the colors without triggering a whole reload.

nikp123

Almost there.

nikp123 · 2024-07-12T15:07:10Z

src/xava.c

+    UNUSED(xavaionot);
+    switch(id) {
+        case XAVA_IONOTIFY_CALLBACK_MAIN:
+            printf("triggered non pywal!\n");


You should remove the debug printf's once you're done debugging ;)

Also for future reference use xavaLog or xavaSpam for these kinds of purposes, it's arguments are the same as printf's.

nikp123 · 2024-07-12T15:14:24Z

src/xava.c

-            xavaLog("ionotify socket closed");
+        case XAVA_IONOTIFY_CALLBACK_PYWAL:
+            printf("triggered pywal!\n");
+            XAVA_CONFIG     *p      = &xava.conf;


This shouldn't be the correct approach, we should trigger a window redraw instead. You're pulling the global xava reference instead of the argument which is undefined behavior.

To redraw the window move the redrawWindow if block from the main loop into its own function, call it xava_window_redraw (just to have a consistent naming scheme) and call it with the xava argument from the callback.

nikp123 · 2024-07-12T15:16:34Z

I made all the changes you asked for except for the dynamic color reload part because I have no idea how to change the colors without triggering a whole reload.

Yeah because it isn't really obvious, I'd never thought of having code that redraws colors outside of the main loop, hence why a bit of architectural elbow grease is required unfortunately.

make-42 · 2024-07-12T15:32:07Z

something like this? it crashes here though (null pointer dereference)
src/xava.c

58    output->func.clear(xava);

nikp123 · 2024-07-12T15:41:56Z

Funnily enough it doesn't for me, can you clear the compile cache, maybe somethings up.

make-42 · 2024-07-12T15:51:30Z

I'm on X11, that might be the issue. the clear function might be erroring out on x11.

nikp123 · 2024-07-12T16:16:57Z

There seems to be an bug with IONotify since it keeps calling non-stop.

make-42 · 2024-07-12T16:23:23Z

Maybe yes

nikp123 · 2024-07-12T16:49:22Z

Ok I found out whats the bug. Firstly you should actually check for an event instead of blindly reloading. Secondly the clear call seg faults on some systems because its supposed to be called from within the main thread (OpenGL moment).

We need an way to signal to the thread that we are redrawing. Exposing redrawWindow might be the solution here.

make-42 · 2024-07-12T16:58:42Z

What do you mean by checking for an event instead of blindly reloading?

make-42 · 2024-07-12T16:59:24Z

oh

make-42 · 2024-07-12T16:59:29Z

i get it

nikp123 · 2024-07-12T16:59:44Z

Anyway I fixed it, sending the patch soon enough

make-42 · 2024-07-12T16:59:52Z

Okay

nikp123 · 2024-07-12T17:00:57Z

fixes.patch.zip

nikp123 · 2024-07-12T17:01:11Z

Apply that patch, push here and I'll rebase

Additionally test out how it works for you.

nikp123 · 2024-07-12T17:04:21Z

Remove the glDrawArrays line I added by accident, it shouldn't be there

nikp123 · 2024-07-12T17:07:44Z

Actually don't, let me squash and merge this.

make-42 · 2024-07-12T17:22:24Z

okay

make-42 · 2024-07-12T17:53:39Z

It doesn't crash but doesn't update the colors either on X11 until a restart.

nikp123 · 2024-07-12T19:03:39Z

It doesn't crash but doesn't update the colors either on X11 until a restart.

Hm weird

nikp123 · 2024-07-12T19:10:18Z

It doesn't crash but doesn't update the colors either on X11 until a restart.

I've tested both X11 and Wayland, neither seem to have the issue on both Cairo and OpenGL

Can you send your config just in case?

make-42 · 2024-07-12T19:47:23Z

config

nikp123 · 2024-07-12T22:40:03Z

config

works even with that config 🤔

make-42 · 2024-07-13T07:35:48Z

I'll do some investigation

make-42 · 2024-07-13T07:41:26Z

Okay. I fixed it. Line 18 of pywal.c it's ".cache" and not ".config" that should be used.

make-42 · 2024-07-13T07:41:57Z

It didn't register the file properly and probably read from xrdb on startup instead.

make-42 · 2024-07-13T07:45:42Z

fix.zip

nikp123 · 2024-07-13T11:50:09Z

@make-42 oh whoops, my bad

sent the patch to master

make-42 · 2024-07-13T11:55:54Z

Haha! Thank you!

make-42-2 added 3 commits July 12, 2024 01:36

feat: add reading from pywal for X11

298b682

feat: formatting

730dbc5

feat: add wayland support

72face0

make-42 mentioned this pull request Jul 11, 2024

[Feature Request] Pull colors from pywal #59

Closed

make-42-2 added 5 commits July 12, 2024 01:52

feat: foreground and background color

b593ee1

feat: foreground and background color

584d859

feat: foreground and background color

6fc4481

feat: foreground and background color

afb822b

feat: foreground and background color

9f11c8c

feat: restart xava on color change

8e5566a

nikp123 requested changes Jul 12, 2024

View reviewed changes

feat: implement most changes

04c9dc8

make-42 added 3 commits July 12, 2024 16:39

feat: remove debug logs

2beafc8

feat: comments

57487a3

feat: check if pywal file exists

22e594c

nikp123 reviewed Jul 12, 2024

View reviewed changes

feat: window redraw on pywal color change

ce98e1d

nikp123 closed this Jul 12, 2024

[Feature Request] Pull colors from pywal #60

[Feature Request] Pull colors from pywal #60

Conversation

make-42 commented Jul 11, 2024 • edited Loading

make-42 commented Jul 12, 2024

nikp123 left a comment

Choose a reason for hiding this comment

nikp123 Jul 12, 2024 • edited Loading

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

make-42 commented Jul 12, 2024

nikp123 commented Jul 12, 2024

make-42 commented Jul 12, 2024

nikp123 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

nikp123 commented Jul 12, 2024

make-42 commented Jul 12, 2024 • edited Loading

nikp123 commented Jul 12, 2024

make-42 commented Jul 12, 2024

nikp123 commented Jul 12, 2024

make-42 commented Jul 12, 2024

nikp123 commented Jul 12, 2024 • edited Loading

make-42 commented Jul 12, 2024 • edited Loading

make-42 commented Jul 12, 2024

make-42 commented Jul 12, 2024

nikp123 commented Jul 12, 2024

make-42 commented Jul 12, 2024

nikp123 commented Jul 12, 2024

nikp123 commented Jul 12, 2024 • edited Loading

nikp123 commented Jul 12, 2024

nikp123 commented Jul 12, 2024

make-42 commented Jul 12, 2024

make-42 commented Jul 12, 2024 • edited Loading

nikp123 commented Jul 12, 2024

nikp123 commented Jul 12, 2024 • edited Loading

make-42 commented Jul 12, 2024

nikp123 commented Jul 12, 2024

make-42 commented Jul 13, 2024

make-42 commented Jul 13, 2024

make-42 commented Jul 13, 2024

make-42 commented Jul 13, 2024

nikp123 commented Jul 13, 2024

make-42 commented Jul 13, 2024 • edited Loading

make-42 commented Jul 11, 2024 •

edited

Loading

nikp123 Jul 12, 2024 •

edited

Loading

make-42 commented Jul 12, 2024 •

edited

Loading

nikp123 commented Jul 12, 2024 •

edited

Loading

make-42 commented Jul 12, 2024 •

edited

Loading

nikp123 commented Jul 12, 2024 •

edited

Loading

make-42 commented Jul 12, 2024 •

edited

Loading

nikp123 commented Jul 12, 2024 •

edited

Loading

make-42 commented Jul 13, 2024 •

edited

Loading