bakkeby / st-flexipatch

An st build with preprocessor directives to decide which patches to include during build time
MIT License
353 stars 108 forks source link

[patch request/question] - swap cursor's background & current character color like alacritty? #10

Closed kiprasmel closed 7 months ago

kiprasmel commented 3 years ago

Alacritty has this and I'd love to have it in st -- once you hover over to some colorful character, the cursor's background becomes that color, and the character becomes transparent or swapped or similar.

This is something similar to ATTR_REVERSE and disabling the spoiler patch, but more like for general editing in say (neo)vim.

image

kiprasmel commented 3 years ago

update: I think I figured something out!

diff --git a/x.c b/x.c
index 696ade9..6fc70a7 100644
--- a/x.c
+++ b/x.c
@@ -1684,8 +1684,9 @@ xdrawcursor(int cx, int cy, Glyph g, int ox, int oy, Glyph og, Line line, int le
            g.fg = defaultfg;
            g.bg = defaultrcs;
        } else {
+           /** swap background, much like alacritty */
+           g.bg = g.fg;
            g.fg = defaultbg;
-           g.bg = defaultcs;
        }
        drawcol = dc.col[g.bg];
    }

this is from the original version of st, not this flexipatch'd one, but still.

I tried it with vim and terminal emacs -- it works!

But neovim somehow wants to crash unless I use this patch instead:

diff --git a/x.c b/x.c
index 696ade9..ba65faa 100644
--- a/x.c
+++ b/x.c
@@ -1684,8 +1684,14 @@ xdrawcursor(int cx, int cy, Glyph g, int ox, int oy, Glyph og, Line line, int le
            g.fg = defaultfg;
            g.bg = defaultrcs;
        } else {
-           g.fg = defaultbg;
-           g.bg = defaultcs;
+           if (g.fg > 259) {
+               g.fg = defaultfg;
+               g.bg = defaultrcs;
+           } else {
+               /** swap background, much like alacritty */
+               g.bg = g.fg;
+               g.fg = defaultbg;
+           }
        }
        drawcol = dc.col[g.bg];
    }

the value of g.fg would skyrocket when launching neovim and a segfault would be thrown. And with this patch, it doesn't even work with neovim - it just won't crash:/ Any ideas?

kiprasmel commented 3 years ago

another update: I think I got it working!

The thing was - the extremely large value of g.fg would cause the segfault, but using the other value (g.bg) as new_drawcol_idx somehow fixed it!

diff --git a/x.c b/x.c
index 696ade9..7ad4c1a 100644
--- a/x.c
+++ b/x.c
@@ -1680,14 +1680,32 @@ xdrawcursor(int cx, int cy, Glyph g, int ox, int oy, Glyph og, Line line, int le
            g.fg = defaultcs;
        }
    } else {
+       uint32_t new_drawcol_idx;
+
        if (selected(cx, cy)) {
            g.fg = defaultfg;
            g.bg = defaultrcs;
-       } else {
+
+           new_drawcol_idx = g.bg;
+       } else if (g.fg <= 259) {
+           /** swap background, much like alacritty */
+           g.bg = g.fg;
            g.fg = defaultbg;
-           g.bg = defaultcs;
+
+           new_drawcol_idx = g.bg;
+       } else {
+           /**
+             * also swap background, but mark `new_drawcol_idx` from the valid value (`fg)
+             * note the number `259` is picked somewhat arbitrarily with some testing,
+             * see https://github.com/bakkeby/st-flexipatch/issues/10#issuecomment-784646647
+           */
+           g.bg = g.fg;
+           g.fg = defaultbg;
+
+           new_drawcol_idx = g.fg;
        }
-       drawcol = dc.col[g.bg];
+
+       drawcol = dc.col[new_drawcol_idx];
    }

    /* draw the new one */

any tips / improvements? I don't particularly like the magic 259 number 😅 Should I submit this to the suckless guys? It looks good ngl:D

edit: I've submitted the patch to the suckless website, hopefully we can figure it out there:D https://git.suckless.org/sites/commit/dd6f420f7ac13c4ab155d23e21aab2a7d3010bf6.html

bakkeby commented 3 years ago

I have to say the end result looks really good.

bakkeby commented 3 years ago

I was curious about the segmentation fault and a few other things, like how the cursor disappears when the window does not have focus.

I ended up with this, perhaps you'd like to try it out.

diff --git a/x.c b/x.c
index 120e495..34242f5 100644
--- a/x.c
+++ b/x.c
@@ -1489,6 +1489,7 @@ void
 xdrawcursor(int cx, int cy, Glyph g, int ox, int oy, Glyph og)
 {
        Color drawcol;
+       XRenderColor colfg;

        /* remove the old cursor */
        if (selected(ox, oy))
@@ -1517,11 +1518,23 @@ xdrawcursor(int cx, int cy, Glyph g, int ox, int oy, Glyph og)
                if (selected(cx, cy)) {
                        g.fg = defaultfg;
                        g.bg = defaultrcs;
+                       drawcol = dc.col[g.bg];
                } else {
-                       g.fg = defaultbg;
-                       g.bg = defaultcs;
+                       if (IS_SET(MODE_FOCUSED)) {
+                               g.bg = g.fg;
+                               g.fg = defaultbg;
+                       }
+
+                       if (IS_TRUECOL(g.fg)) {
+                               colfg.alpha = 0xffff;
+                               colfg.red = TRUERED(g.fg);
+                               colfg.green = TRUEGREEN(g.fg);
+                               colfg.blue = TRUEBLUE(g.fg);
+                               XftColorAllocValue(xw.dpy, xw.vis, xw.cmap, &colfg, &drawcol);
+                       } else {
+                               drawcol = dc.col[g.fg];
+                       }
                }
-               drawcol = dc.col[g.bg];
        }

        /* draw the new one */

I don't know the st code that well, but this seems to work correctly.

kiprasmel commented 3 years ago

oh wow that's so much better, thanks!!

The only problem I had was that my cursor was hidden if it was in INSERT mode and my background was dark (it worked fine when the background was light). This is what I came up with, adding on top of your patch (I just added another if/else with the IS_TRUECOL check for the g.bg case.

diff --git a/x.c b/x.c
index 696ade9..e790f23 100644
--- a/x.c
+++ b/x.c
@@ -1652,6 +1652,7 @@ void
 xdrawcursor(int cx, int cy, Glyph g, int ox, int oy, Glyph og, Line line, int len)
 {
    Color drawcol;
+   XRenderColor colfg;

    /* remove the old cursor */
    if (selected(ox, oy))
@@ -1683,11 +1684,43 @@ xdrawcursor(int cx, int cy, Glyph g, int ox, int oy, Glyph og, Line line, int le
        if (selected(cx, cy)) {
            g.fg = defaultfg;
            g.bg = defaultrcs;
+           drawcol = dc.col[g.bg];
        } else {
-           g.fg = defaultbg;
-           g.bg = defaultcs;
+           /** this is the main part of the dynamic cursor color */
+           if (IS_SET(MODE_FOCUSED)) {
+               g.bg = g.fg;
+               g.fg = defaultbg;
+           }
+
+           /**
+            * the following 2 sections are identical,
+            * they differ only by either using `g.fg` or `g.bg`,
+            * and it depends on what background+foreground
+            * the user has configured (light+dark vs dark+light)
+            *
+            * otherwise, in one of the cases, the cursor will be invisible
+           */
+
+           if (IS_TRUECOL(g.fg)) {
+               colfg.alpha = 0xffff;
+               colfg.red = TRUERED(g.fg);
+               colfg.green = TRUEGREEN(g.fg);
+               colfg.blue = TRUEBLUE(g.fg);
+               XftColorAllocValue(xw.dpy, xw.vis, xw.cmap, &colfg, &drawcol);
+           } else {
+               drawcol = dc.col[g.fg];
+           }
+           
+           if (IS_TRUECOL(g.bg)) {
+               colfg.alpha = 0xffff;
+               colfg.red = TRUERED(g.bg);
+               colfg.green = TRUEGREEN(g.bg);
+               colfg.blue = TRUEBLUE(g.bg);
+               XftColorAllocValue(xw.dpy, xw.vis, xw.cmap, &colfg, &drawcol);
+           } else {
+               drawcol = dc.col[g.bg];
+           }
        }
-       drawcol = dc.col[g.bg];
    }

    /* draw the new one */

seems to work just fine for both light & dark backgrounds! I will submit this update to the st guys and hopefully we can iterate further there, but your help is very much appreciated, just like now!<3

P.S. debugging with gdb is actually really fun haha

edit: https://git.suckless.org/sites/commit/0c8b68e2e53eac6537da5585fe115eb3e70a4bf4.html 👀

bakkeby commented 3 years ago

I will submit this update to the st guys and hopefully we can iterate further there

It's better to have a ready patch when submitting to the suckless site, the patches area is more of a user space and it is not to my knowleddge a place where the suckless guys will discuss things with you.

In your version the drawcol variable is being overwritten by the background color, so setting it to the foreground color first is redundant. I think we can refactor this further.

bakkeby commented 3 years ago

This should be identical to what you have above.

diff --git a/x.c b/x.c
index 120e495..ed627e8 100644
--- a/x.c
+++ b/x.c
@@ -1489,6 +1489,7 @@ void
 xdrawcursor(int cx, int cy, Glyph g, int ox, int oy, Glyph og)
 {
        Color drawcol;
+       XRenderColor colbg;

        /* remove the old cursor */
        if (selected(ox, oy))
@@ -1518,10 +1519,19 @@ xdrawcursor(int cx, int cy, Glyph g, int ox, int oy, Glyph og)
                        g.fg = defaultfg;
                        g.bg = defaultrcs;
                } else {
+                       g.bg = g.fg;
                        g.fg = defaultbg;
-                       g.bg = defaultcs;
                }
-               drawcol = dc.col[g.bg];
+
+               if (IS_TRUECOL(g.bg)) {
+                       colbg.alpha = 0xffff;
+                       colbg.red = TRUERED(g.bg);
+                       colbg.green = TRUEGREEN(g.bg);
+                       colbg.blue = TRUEBLUE(g.bg);
+                       XftColorAllocValue(xw.dpy, xw.vis, xw.cmap, &colbg, &drawcol);
+               } else {
+                       drawcol = dc.col[g.bg];
+               }
        }

        /* draw the new one */
kiprasmel commented 3 years ago

It's better to have a ready patch when submitting to the suckless site, the patches area is more of a user space and it is not to my knowleddge a place where the suckless guys will discuss things with you.

okay, I wasn't aware, I'm more used to the github's pull-request method - good to know.

In your version the drawcol variable is being overwritten by the background color, so setting it to the foreground color first is redundant. I think we can refactor this further.

oh, yes, I figured that it would be somewhat fine since I didn't encounter a situation where both g.bg and g.fg were true colors, but yes, it would get overridden.

I tried the refactored version and it's good -- thank you once again!

edit: https://git.suckless.org/sites/commit/76b6a6d322059902c4651863d1eef5ca85da680b.html and https://git.suckless.org/sites/log.html

I hope they'll just squash the commits at this point 😅

AtifChy commented 3 years ago

hightlighted letters are unreadable image

bakkeby commented 3 years ago

@AtifChy, I'd assume that this would be due to configuration for whatever program you are using.

AtifChy commented 3 years ago

works fine on alacritty. that was inside vim.

bakkeby commented 3 years ago

@AtifChy, I can't help you if I can't replicate the issue.

Barbaross93 commented 3 years ago

Out of curiousity, does this require the block style cursor? I'm using the underline style and it doesn't seem to work as intended, i.e. the cursor color is the same as the current text selection

kyx0r commented 3 years ago

@Barbarossa93 That's intended behavior. What is your expectation?

kyx0r commented 3 years ago

@Barbarossa93 You might need this patch: http://st.suckless.org/patches/bold-is-not-bright/st-bold-is-not-bright-20190127-3be4cf1.diff

if the cursor color does not match, because st changes bold colors by default.

Barbaross93 commented 3 years ago

To be honest, now that I think about it I'm not sure what I'd expect to happen. I guess I just wanted to make sure that what I'm seeing is expected. I do have that patch applied, the underline cursor is the same as the text color in my case

bakkeby commented 3 years ago

The expectation here is that the cursor has the same color as the font.

If you try to select text then the selection should have the same color as the font color and the selected text should take the color of the background.

If you have a block cursor and move the cursor on top of existing text then the same should happen. If you have an underscore cursor then surprisingly it doesn't screw things up.

To answer your question this patch is intended for the block style cursor, but it doesn't seem to require it.

I also cross checked and this also works with blinking cursors.

I believe I also found a bug in st. If you have an underline or bar cursor, and you use vim to edit a file then the cursor changes to a block (default vim configuration). When you exit vim then that cursor remains on the prompt instead of your underline or bar cursor. None of the other terminal emulators that I tested have this problem.

bakkeby commented 3 years ago

I believe I also found a bug in st. If you have an underline or bar cursor, and you use vim to edit a file then the cursor changes to a block (default vim configuration). When you exit vim then that cursor remains on the prompt instead of your underline or bar cursor. None of the other terminal emulators that I tested have this problem.

Committed a proposed fix for that.

Barbaross93 commented 3 years ago

Awesome, thanks for the clarification!

eeeXun commented 3 years ago

hightlighted letters are unreadable image

Have the same problem too. image If I was in vim diff mode. I can't see the cursor. image And this should look like in alacritty.

bakkeby commented 3 years ago

@eeeXun can you try this change and see if that works in all situations for you?

diff --git a/x.c b/x.c
index 2767045..6c8e165 100644
--- a/x.c
+++ b/x.c
@@ -2028,8 +2028,9 @@ xdrawcursor(int cx, int cy, Glyph g, int ox, int oy, Glyph og)
                        g.bg = defaultrcs;
                } else {
                        #if DYNAMIC_CURSOR_COLOR_PATCH
+                       unsigned int tmpcol = g.bg;
                        g.bg = g.fg;
-                       g.fg = defaultbg;
+                       g.fg = tmpcol;
                        #else
                        g.fg = defaultbg;
                        g.bg = defaultcs;
(END)
eeeXun commented 3 years ago

@bakkeby No, it seems not work.Still can't see the cursor

eeeXun commented 3 years ago

Might be gruvbox colorscheme produce the problem. If I change to other colorscheme, it's much better.

bakkeby commented 3 years ago

Yes I don't know, I would have thought the above change might have done the trick. Don't remember why fg is using defaultbg in this patch. Looks alright for me in vimdiff.

eeeXun commented 3 years ago

image If I was in vim diff mode. I can't see the cursor.

I found that the bg is black, and fg is yellow in this picture. That's why it doesn't take effect. So weird

kiprasmel commented 3 years ago

@bakkeby

@eeeXun can you try this change and see if that works in all situations for you?

@eeeXun

@bakkeby No, it seems not work.Still can't see the cursor

should we apply the @bakkeby's proposed patch for tmpcol regardless?

I too do not remember why we used the defaultbg; perhaps it didn't matter (it wouldn't if g.bg was always set to defaultbg)? I tried a few files and there was no difference, but we might need more testing.

paniash commented 3 years ago

Yes I don't know, I would have thought the above change might have done the trick. Don't remember why fg is using defaultbg in this patch. Looks alright for me in vimdiff.

The issue with vimdiff does persist as mentioned by @eeeXun. Can't see the cursor at all.

paniash commented 3 years ago

The issue with vimdiff does persist as mentioned by @eeeXun. Can't see the cursor at all.

Weirdly enough, the issue only persists when gruvbox is used as colorscheme. The default colorscheme works fine for me in vimdiff.

bakkeby commented 3 years ago

Using the gruvbox color scheme that should really be the same as using theme.sh and running:

$ theme.sh gruvbox
$ theme.sh gruvbox-dark

or adding the theme to your ~/.Xresources file.

If you are manually applying the gruvbox patch to get this color scheme then those patches do play around with the default color settings:

-unsigned int defaultfg = 7;
-unsigned int defaultbg = 0;
-static unsigned int defaultcs = 256;
+unsigned int defaultfg = 15;
+unsigned int defaultbg = 0;
+static unsigned int defaultcs = 15;
 static unsigned int defaultrcs = 257;

The above would have the effect that the cursor would not be visible for some highlights in vimdiff, it doesn't have anything to do with the gruvbox color scheme in itself.

JuanScerriE commented 3 years ago

Hello,

First of all thanks for such a great patch. It really adds to the experience. I decided that I would take it upon my self to try and solve the issue described by @eeeXun.

After some debugging I found out the following:

The problem is effectively generated by these few lines in the function xdrawglyphfontspecs(). 1627494582 When you move into a Glyph or Cell whose background is colored like you would in a diff file, base.mode is set to 32 which is the value of ATTR_REVERSE. The same thing happens when leaving the colored Glyph so the cursor returns to normal.

I tried to depict the above description as effectively as I possibly could using images. 1627496657 1627496743 1627496803

Now as we can see there is another Glyph variable og.mode that is present within the xdrawcursor() function which coincidentally has the same value as base.mode. So I just did the following.

1627497902

{Ignore the fprints. They where used for debugging.} This solves the issue. Also I explicitly swapped the values of the g.fg and g.bg, as is done in Alacritty.

1627498012 https://github.com/alacritty/alacritty/blob/master/alacritty/src/display/content.rs

Any feedback is greatly appreciated.

kiprasmel commented 3 years ago

Hey @JuanScerriE, thanks!

Could you possibly clarify your attempted patch? I am trying to re-create it but it isn't exactly clear where you placed the following code and if there was anything else you changed?

if (!(og.mode & ATTR_REVERSE)) {
    unsigned long tmpcol = g.bg;
    g.bg = g.fg;
    g.fg = tmpcol;
}

and is it not the same as what @bakkeby has suggested previously?

@bakkeby:

@eeeXun can you try this change and see if that works in all situations for you?

diff --git a/x.c b/x.c
index 2767045..6c8e165 100644
--- a/x.c
+++ b/x.c
@@ -2028,8 +2028,9 @@ xdrawcursor(int cx, int cy, Glyph g, int ox, int oy, Glyph og)
                        g.bg = defaultrcs;
                } else {
                        #if DYNAMIC_CURSOR_COLOR_PATCH
+                       unsigned int tmpcol = g.bg;
                        g.bg = g.fg;
-                       g.fg = defaultbg;
+                       g.fg = tmpcol;
                        #else
                        g.fg = defaultbg;
                        g.bg = defaultcs;
(END)

and @bakkeby - could you help me recall if we have tried incorporating this patch yet?

bakkeby commented 3 years ago

@kiprasmel this sounds like something different to what I proposed.

@eeeXun you have put a lot of effort into debugging this, kudos for that. Maybe it is easier if you show a patch for a bare st that adds all the changes that are necessary to swap the cursor's background and foreground colors? I tried adding that if statement in a random place in the xdrawcursor function but it just makes the cursor disappear completely for me.

JuanScerriE commented 3 years ago

Sorry, if I was not clear enough. Here is a diff file of the modification.

Common subdirectories: st-fresh/.git and st/.git
Binary files st-fresh/st and st/st differ
diff '--color=auto' -up st-fresh/x.c st/x.c
--- st-fresh/x.c    2021-07-21 22:24:34.964357693 +0200
+++ st/x.c  2021-07-29 14:43:33.584141458 +0200
@@ -1491,6 +1491,7 @@ void
 xdrawcursor(int cx, int cy, Glyph g, int ox, int oy, Glyph og)
 {
    Color drawcol;
+   XRenderColor colbg;

    /* remove the old cursor */
    if (selected(ox, oy))
@@ -1520,10 +1521,27 @@ xdrawcursor(int cx, int cy, Glyph g, int
            g.fg = defaultfg;
            g.bg = defaultrcs;
        } else {
-           g.fg = defaultbg;
-           g.bg = defaultcs;
+           /** this is the main part of the dynamic cursor color patch */
+           if (!(og.mode & ATTR_REVERSE)) {
+               unsigned long col = g.bg;
+               g.bg = g.fg;
+               g.fg = col;
+           }
+       }
+
+       /**
+        * and this is the second part of the dynamic cursor color patch.
+        * it handles the `drawcol` variable
+       */
+       if (IS_TRUECOL(g.bg)) {
+           colbg.alpha = 0xffff;
+           colbg.red = TRUERED(g.bg);
+           colbg.green = TRUEGREEN(g.bg);
+           colbg.blue = TRUEBLUE(g.bg);
+           XftColorAllocValue(xw.dpy, xw.vis, xw.cmap, &colbg, &drawcol);
+       } else {
+           drawcol = dc.col[g.bg];
        }
-       drawcol = dc.col[g.bg];
    }

    /* draw the new one */
JuanScerriE commented 3 years ago

1627563500

This works for me. I think it is also good to clarify that I am using Neovim. The funny thing is that when st encounters a situation as you would in a diff file. The green and red colors are actually treated as the foreground instead of the background. I do not know why yet.

bakkeby commented 3 years ago

As far as I can see this works fine. I am just trying to replicate the issue with vimdiff and the gruvbox theme to confirm.

JuanScerriE commented 3 years ago

Sorry could you clarify is it working as intended? Or was this not an issue in the first place in some cases? The patch is identical to what what was already proposed apart from the the if statement and explicitly swapping the foreground and background.

eeeXun commented 3 years ago

@JuanScerriE Amazing, it work as expected.

JuanScerriE commented 3 years ago

Thanks! It is a very small change but I believe that the problem runs deeper then this to be honest. I am not sure if it is my brain playing tricks on me but I think there is a slight delay. I think it has something to do with the way st interprets foregrounds and backgrounds. Unfortunately, I have not managed to find where st gets that information explicitly in the source code. For now however, it is a good fix. I will keep you update if I find anything else.

bakkeby commented 3 years ago

It's not that straightforward, st gets the information from the shell so it's all about interpreting escape codes.

I have been trying to reproduce the vimdiff issue without luck. On the same note I haven't found any case where this change breaks anything or causes any odd display issues.

I'd say let's just merge this in and follow up if someone stumbles upon any new issues.

humky commented 2 years ago

The patch to fix vim search highlight doesn't work well with close to zero window opacity: about 40%: trans2 0%: trans1

bakkeby commented 2 years ago

@humky I don't know if this has anything specifically to do with this. It may an issue with the alpha patch itself.

If I apply the alpha patch on top of 0.8.2, set the alpha value to 0 and try to select something all the text disappears.

image

humky commented 2 years ago

Cursor color (both dynamic and not) with very low alpha are buggy in general, I could get the cursor itself working with

g.bg = g.fg;
g.fg = bg;

(For dynamic cursor, without the vim search fix) Selections are still buggy, but cursor works for light/dark theme with any transparency. Still need to fix vim search and selections. I starting to think that it will be required to use custom selection color patch to set selection fg to bg; as well, I'm not sure what's happening here with alpha. Some tweaks can be made in vimrc itself, for search colors. Lots of moving pieces here, and may be theme/environment specific. I must also point out that I use my own build, so I can't provide exact code snippets, but the problem with cursor color and low alpha is universal for all st builds I've tried.

humky commented 2 years ago

I was able to get selection working with this:

index 92f3c51..76aef27 100644
--- a/x.c
+++ b/x.c
@@ -1753,9 +1753,8 @@ xdrawglyphfontspecs(const XftGlyphFontSpec *specs, Glyph base, int len, int x, i
        }

        if (base.mode & ATTR_REVERSE) {
-               temp = fg;
-               fg = bg;
-               bg = temp;
+        fg = &dc.col[0];
+        bg = &dc.col[base.fg];
        }

        if (base.mode & ATTR_BLINK && win.mode & MODE_BLINK)

The only issue remains, vim search. If I use the fix from above it screws the cursor like in my screenshot. The cursor becomes faint even at 50% opacity which is not a crazy transparency, on dark backgrounds I can totally see myself going that low. I tried setting colors in vimrc but didn't like the effect. Sorry if stealing the thread somewhat, I think it's mostly related. This page is high in search rankings and since people are often doing their own builds of st, my findings might be useful to someone.

bakkeby commented 2 years ago

Hmm, yes maybe. This didn't quite work for me.

        if (base.mode & ATTR_REVERSE) {
-               temp = fg;
-               fg = bg;
-               bg = temp;
+        fg = &dc.col[0];
+        bg = &dc.col[base.fg];
        }

The 0 here would be referencing defaultbg I presume.

I had better luck with this:

-               temp = fg;
-               fg = bg;
-               bg = temp;
+               colfg.alpha = 0xffff;
+               colfg.red = bg->color.red;
+               colfg.green = bg->color.green;
+               colfg.blue = bg->color.blue;
+               XftColorAllocValue(xw.dpy, xw.vis, xw.cmap, &colfg, &truefg);
+
+               colbg.alpha = 0xffff;
+               colbg.red = fg->color.red;
+               colbg.green = fg->color.green;
+               colbg.blue = fg->color.blue;
+               XftColorAllocValue(xw.dpy, xw.vis, xw.cmap, &colbg, &truebg);
+
+               fg = &truefg;
+               bg = &truebg;

Probably worth having someone who actually knows how this stuff works to look over this :) The above was specifically in relation to having an alpha value of 0.0 and selecting text in the terminal. I have a powerline prompt where the text is not visible still.

As for the vim search issue, can you try this change and see if that works for you?

@@ -2376,6 +2408,7 @@ xdrawcursor(int cx, int cy, Glyph g, int ox, int oy, Glyph og)
                        unsigned int tmpcol = g.bg;
                        g.bg = g.fg;
                        g.fg = tmpcol;
+                       g.fg |= 0xFF000000;
                }

                if (IS_TRUECOL(g.bg)) {

Alternatively:

@@ -2373,7 +2405,7 @@ xdrawcursor(int cx, int cy, Glyph g, int ox, int oy, Glyph og)
                drawcol = dc.col[g.bg];
                #else
                else if (!(og.mode & ATTR_REVERSE)) {
-                       unsigned int tmpcol = g.bg;
+                       unsigned int tmpcol = g.bg | 0xFF000000;
                        g.bg = g.fg;
                        g.fg = tmpcol;
                }
humky commented 2 years ago

defaultbg is 258 in my config, 0 is theme's bg. your patch has more contrast, and it fixes the powerline selection for me. I don't really use powerline so I've tried some random ones for bash: trans1 I know powerline symbols are messed up (common issue), I did not debug that, but selection is working. in vim: trans3 Unfortunately vim search is only mildly better trans2


I will disable highlight background altogether for now, decent workaround I think:

trans1-2 trans1-1

highlight Search cterm=bold ctermfg=yellow ctermbg=none
sahinakkaya commented 1 year ago

@eeeXun can you try this change and see if that works in all situations for you?

diff --git a/x.c b/x.c
index 2767045..6c8e165 100644
--- a/x.c
+++ b/x.c
@@ -2028,8 +2028,9 @@ xdrawcursor(int cx, int cy, Glyph g, int ox, int oy, Glyph og)
                        g.bg = defaultrcs;
                } else {
                        #if DYNAMIC_CURSOR_COLOR_PATCH
+                       unsigned int tmpcol = g.bg;
                        g.bg = g.fg;
-                       g.fg = defaultbg;
+                       g.fg = tmpcol;
                        #else
                        g.fg = defaultbg;
                        g.bg = defaultcs;
(END)

~I can confirm that this works for me~ (see update below). Here is what it looks like with plain dynamic cursor color patch: hlgroup

And here is what it looks like after applying the mentioned fix: hlgroupafter

Here is the patch I applied:


diff --git a/x.c b/x.c
index 9cb65b4..54339c0 100644
--- a/x.c
+++ b/x.c
@@ -2138,8 +2138,9 @@ xdrawcursor(int cx, int cy, Glyph g, int ox, int oy, Glyph og, Line line, int le
            g.bg = defaultrcs;
        } else {
            /** this is the main part of the dynamic cursor color patch */
+           unsigned int tmpcol = g.bg;
            g.bg = g.fg;
-           g.fg = defaultbg;
+           g.fg = tmpcol;
        }

        /**

Update: Apparently, it doesn't work well with light backgrounds. Here is what it looks like with light backgrounds: image

Update 2: I tried https://github.com/bakkeby/st-flexipatch/issues/10#issuecomment-889120271 and it worked!

eeeXun commented 1 year ago

https://github.com/bakkeby/st-flexipatch/issues/10#issuecomment-889120271 this work for me.

sahinakkaya commented 1 year ago

@bakkeby you might consider submitting the latest working patch to st's dynamic cursor color patch.

bakkeby commented 1 year ago

@sahinakkayadev pushed that through here: https://git.suckless.org/sites/commit/40c063c4c2770ef5f561413a8f02ae8182040253.html

bakkeby commented 7 months ago

Don't think there was anything left outstanding on this one so going to mark this as closed.