Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding reflow patch #120

Merged
merged 5 commits into from
Mar 13, 2024
Merged

Adding reflow patch #120

merged 5 commits into from
Mar 13, 2024

Conversation

bakkeby
Copy link
Owner

@bakkeby bakkeby commented Mar 11, 2024

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

Closes #34

@veltza
Copy link
Contributor

veltza commented Mar 12, 2024

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:

  • Cannot compile when only the reflow and keyboard select patches are enabled.

  • highlightfg and highlightbg variables are missing from config.def.h (improved keyboard select patch needs these)

  • Sixels disappear from the scrollback when reflow is enabled. This needs to be investigated.

  • (Improved) keyboard select does not copy anything to the clipboard unless the clipboard patch is also enabled. This could be fixed like this:

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
Copy link
Owner Author

bakkeby commented Mar 12, 2024

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
Copy link
Contributor

veltza commented Mar 12, 2024

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
Copy link
Owner Author

bakkeby commented Mar 12, 2024

Done. Thanks for noticing.

… scrollback patch, so include the mouse bindings as well.
@bakkeby
Copy link
Owner Author

bakkeby commented Mar 13, 2024

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.

@bakkeby bakkeby merged commit 3b87b07 into master Mar 13, 2024
@bakkeby bakkeby deleted the reflow branch March 13, 2024 09:34
@veltza
Copy link
Contributor

veltza commented Mar 13, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[request] Add patch column patch
2 participants