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

Adding reflow patch #120

Closed bakkeby closed 7 months ago

bakkeby commented 7 months ago

This adds the reflow "patch" based on @veltza's implementation.

Closes #34

veltza commented 7 months ago

Now that the reflow and improved keyboard select patches are moving to st-flexipatch, st-sx has very little reason to exist anymore unless I implement the kitty image protocol as well. lol

But we're not there yet because I found the following issues in this PR:

diff --git a/patch/keyboardselect_reflow_st.c b/patch/keyboardselect_reflow_st.c
index e6b8364..57bfedc 100644
--- a/patch/keyboardselect_reflow_st.c
+++ b/patch/keyboardselect_reflow_st.c
@@ -193,6 +193,10 @@ kbds_copytoclipboard(void)
        selextend(kbds_c.x, kbds_c.y, kbds_seltype, 1);
    }
    xsetsel(getsel());
+
+   #if !CLIPBOARD_PATCH
+   xclipcopy();
+   #endif // !CLIPBOARD_PATCH
 }

 void
bakkeby commented 7 months ago

I'll look into the sixels disappearing from scrollback.

st-sx has very little reason to exist anymore

I would disagree :). I see it as a fine example implementation.

Due to the general approach st-flexipatch is inevitably a platter of spaghetti code and having a dedicated build sounds more fun to work with. Maybe that will inspire me to roll my own at some point.

veltza commented 7 months ago

I just noticed that neither the reflow patch nor st-sx use the ATTR_SELECTED attribute. The attribute came from the scrollback-reflow patch, but it's unused there as well.

So, you might want to remove the REFLOW_PATCH condition here to avoid confusion:

    #if SELECTION_COLORS_PATCH || REFLOW_PATCH
    ATTR_SELECTED       = 1 << 12,
    #endif // SELECTION_COLORS_PATCH | REFLOW_PATCH
bakkeby commented 7 months ago

Done. Thanks for noticing.

bakkeby commented 7 months ago

This seems stable enough. I tried with a combination of alpha, anysize, boxdraw, clipboard, keyboardselect, ligatures, reflow, scrollback, selection colors, sixel, vertcenter and wide glyphs patches and I didn't run into any compilation errors or the terminal misbehaving.

I am aware that the keyboard select patch is not actually reflow specific; it is more of an upgrade (or it could be considered as a different patch variant). Didn't want to invest time in backporting that to work with pre-reflow resizing and scrollback behaviour hence added it as a reflow specific option.

Going to go forward and merge this.

veltza commented 7 months ago

I am aware that the keyboard select patch is not actually reflow specific; it is more of an upgrade (or it could be considered as a different patch variant). Didn't want to invest time in backporting that to work with pre-reflow resizing and scrollback behaviour hence added it as a reflow specific option.

Technically, it should be compatible, with minor changes, with the scrollback patch, since they use a very similar scrollback structure with the reflow. In fact, I wrote an early version of the improved variant to work with the scrollback patch. So, if there is demand, I can take a look at it too.