Skip to content

fix: handle zero terminal cell height#1184

Merged
andrinoff merged 1 commit intofloatpane:masterfrom
Haroka-74:fix/issue-865
Apr 28, 2026
Merged

fix: handle zero terminal cell height#1184
andrinoff merged 1 commit intofloatpane:masterfrom
Haroka-74:fix/issue-865

Conversation

@Haroka-74
Copy link
Copy Markdown
Contributor

What?

Added a check for cellHeight == 0 in the imageRows function within view/html.go. If the terminal height cannot be determined, it now defaults to a standard 16px fallback.

Why?

When running in terminals that don't report cell size via ioctl, getTerminalCellSize() returns 0. This previously caused a "division by zero" runtime panic during the row calculation for inline images: (h + cellHeight - 1) / cellHeight.

Closes #865

@Haroka-74 Haroka-74 requested a review from a team as a code owner April 28, 2026 08:59
Copy link
Copy Markdown
Member

@floatpanebot floatpanebot left a comment

Choose a reason for hiding this comment

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

Hi @Haroka-74! Please fix the following issues with your PR:

  • Title: Is too long (56 characters). The PR title must be strictly under 40 characters.

@github-actions github-actions Bot added the bug Something isn't working label Apr 28, 2026
@Haroka-74 Haroka-74 changed the title fix(view): handle zero terminal cell height in imageRows fix(view): handle zero terminal cell height Apr 28, 2026
Copy link
Copy Markdown
Member

@floatpanebot floatpanebot left a comment

Choose a reason for hiding this comment

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

Hi @Haroka-74! Please fix the following issues with your PR:

  • Title: Is too long (43 characters). The PR title must be strictly under 40 characters.

@LeaWhoCodes LeaWhoCodes changed the title fix(view): handle zero terminal cell height fix: handle zero terminal cell height Apr 28, 2026
@floatpanebot floatpanebot dismissed stale reviews from themself April 28, 2026 09:01

Formatting issues have been resolved. Thank you!

LeaWhoCodes
LeaWhoCodes previously approved these changes Apr 28, 2026
Copy link
Copy Markdown
Member

@LeaWhoCodes LeaWhoCodes left a comment

Choose a reason for hiding this comment

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

lgtm

@LeaWhoCodes LeaWhoCodes self-requested a review April 28, 2026 09:04
@LeaWhoCodes
Copy link
Copy Markdown
Member

@Haroka-74, actually, found a piece of code

func getTerminalCellSize() int {
	const defaultCellHeight = 18

	// Try stdout, stdin, stderr, then /dev/tty as last resort
	fds := []int{int(os.Stdout.Fd()), int(os.Stdin.Fd()), int(os.Stderr.Fd())}

	for _, fd := range fds {
		if cellHeight := getCellHeightFromFd(fd); cellHeight > 0 {
			return cellHeight
		}
	}

	// Try /dev/tty directly - this works even when stdio is redirected (e.g., in Bubble Tea)
	if tty, err := os.Open("/dev/tty"); err == nil {
		defer tty.Close()
		if cellHeight := getCellHeightFromFd(int(tty.Fd())); cellHeight > 0 {
			return cellHeight
		}
	}

	debugImageProtocol("using default cell height: %d pixels", defaultCellHeight)
	return defaultCellHeight
}

html.go:28-50

where we do have a fallback value. @andrinoff will have to check this out later, he is extremely busy right now, so no promises

@LeaWhoCodes LeaWhoCodes dismissed their stale review April 28, 2026 09:06

needs attention

@LeaWhoCodes LeaWhoCodes requested a review from andrinoff April 28, 2026 09:06
@Haroka-74
Copy link
Copy Markdown
Contributor Author

@LeaWhoCodes

Good point, I didn’t notice const defaultCellHeight = 18.

@LeaWhoCodes
Copy link
Copy Markdown
Member

@LeaWhoCodes

Good point, I didn’t notice const defaultCellHeight = 18.

this might still be a good fix, if it is determined that we can find a 0 in the function

Copy link
Copy Markdown
Member

@andrinoff andrinoff left a comment

Choose a reason for hiding this comment

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

Good fallback in case the system says the cellHeight = 0.

lgtm

@andrinoff
Copy link
Copy Markdown
Member

/approve

@andrinoff andrinoff merged commit 7c91cb4 into floatpane:master Apr 28, 2026
18 of 20 checks passed
Copy link
Copy Markdown
Member

@floatpanebot floatpanebot left a comment

Choose a reason for hiding this comment

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

Approved on behalf of @andrinoff via /approve command.

@Haroka-74 Haroka-74 deleted the fix/issue-865 branch April 28, 2026 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: Division by zero potential in imageRows when cellHeight is 0

4 participants