GhostNaN / mpvpaper

A video wallpaper program for wlroots based wayland compositors.
GNU General Public License v3.0
765 stars 24 forks source link

added cflogprinter #4

Closed ghost closed 4 years ago

ghost commented 4 years ago

87249b7

GhostNaN commented 4 years ago
Well... this is interesting.

Approved checklist:


What's the difference between "bad error" or just "bad"? Shouldn't it just be "cflp_error" and then send that to stderr?

void cflp_error(char *msg, ...) {
    printf("%s[-] ", RED);

    va_list arg;

    va_start (arg, msg);
    vfprintf (stderr, msg, arg); //print to stderr instead of stdout!
    va_end (arg);

    printf("%s\n", RESET);

}

https://github.com/GhostNaN/mpvpaper/commit/87249b7166564b00bc556a8835db02969203e286#diff-f04cc8d345258a64b0a26daa94b868e1R29

src/paper.c

Inconsistencies between "cflp_bad_error" and "cflp_bad" as I mentioned. So just use "cflp_error".


This was wrongly changed to include "background" when it's supposed to point to the shell layer( layer_name ) being used like "top", "overlay" or "bottom".

cflp_info("Shell layer %s set", layer_name);

https://github.com/GhostNaN/mpvpaper/commit/87249b7166564b00bc556a8835db02969203e286#diff-b417d3bd2cbc0ea819345ce8b0f48bcaR166


Small nit pick with the loaded video message. It makes more sense to be a info message.

cflp_info("Loaded %s", video_path);

https://github.com/GhostNaN/mpvpaper/commit/87249b7166564b00bc556a8835db02969203e286#diff-b417d3bd2cbc0ea819345ce8b0f48bcaR266

Final Notes

I can cleanup the rest after these modifications are made. Overall I wasn't expecting this kind of pull request. It's just eye candy, but I'll allow it.

ghost commented 4 years ago

@GhostNaN

It is just me or does a warning feel more like a yellow to me.

are you referring to my picture or have you tried it yourself in your terminal? maybe it's your terminal colors/theme?

What's the difference between "bad error" or just "bad"? Shouldn't it just be "cflp_error" and then send that to stderr?

Inconsistencies between "cflp_bad_error" and "cflp_bad" as I mentioned. So just use "cflp_error".

In the beginning there was only _cflpbad. But then I saw this line (below) and then added _cflp_baderror because I saw you printing out to stderr....

https://github.com/GhostNaN/mpvpaper/blob/21bb932ffb7bcd87397637c281eeb32ef92d0a6e/src/paper.c#L129

But I can simply rename it to _cflperror

This was wrongly changed to include "background" when it's supposed to point to the shell layer( layer_name ) being used like "top", "overlay" or "bottom".

No problem, I can fix it....

Small nit pick with the loaded video message. It makes more sense to be a info message.

You're right....