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

bufwindow: Fix slice creation with wordwrap active in case of zero size split #3056

Closed
wants to merge 1 commit into from

Conversation

JoeKar
Copy link
Collaborator

@JoeKar JoeKar commented Nov 29, 2023

The caption of the slice needs to have a minimum of 1 and thus 0 isn't possible.

Fixes #3052

@@ -601,7 +601,7 @@ func (w *BufWindow) displayBuffer() {
}

var word []glyph
if wordwrap {
if wordwrap && w.bufWidth > 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guilty of this bug, I admit it.

But it seems the more correct fix is:

--- a/internal/display/bufwindow.go
+++ b/internal/display/bufwindow.go
@@ -152,6 +152,9 @@ func (w *BufWindow) updateDisplayInfo() {
 	if w.Buf.Settings["scrollbar"].(bool) && w.Buf.LinesNum() > w.Height {
 		w.bufWidth--
 	}
+	if w.bufWidth < 0 {
+		w.bufWidth = 0
+	}
 
 	if w.bufWidth != prevBufWidth && w.Buf.Settings["softwrap"].(bool) {
 		for _, c := range w.Buf.GetCursors() {

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, the crash actually happens not when bufWidth becomes zero but when it becomes negative. (Slice cap can be zero, but it cannot be negative.)

And BTW, the root cause of it is that gutterOffset is bigger than the window width, which also causes the ruler overwriting the other split pane (as you probably noticed). I'm now working on the fix for that, which probably will also fix this crash.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it is: #3069

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[...] as you probably noticed [...]

Yes I did. Any way, we go with yours.

@JoeKar
Copy link
Collaborator Author

JoeKar commented Dec 3, 2023

Closed in favor of #3069, since it solves more issues in a much better way.
Thank you @dmaluka!

@JoeKar JoeKar closed this Dec 3, 2023
@JoeKar JoeKar deleted the fix/crash-split-wordwrap branch December 3, 2023 11:41
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.

Crash when pane/(v)split widthe is smaller than ruler size
2 participants