Skip to content

Enhance terminal size detection for multiple outputs - #22276

Merged
jay merged 1 commit into
curl:masterfrom
alankingpl0-oss:patch-2
Jul 24, 2026
Merged

Enhance terminal size detection for multiple outputs#22276
jay merged 1 commit into
curl:masterfrom
alankingpl0-oss:patch-2

Conversation

@alankingpl0-oss

Copy link
Copy Markdown
Contributor

Added additional checks for terminal size using STDERR and STDOUT.

@testclutch

Copy link
Copy Markdown

Analysis of PR #22276 at c8aabda4:

Test 311 failed, but it has been 2.1% flaky lately, so it's probably NOT a fault of the PR. Note that this test has failed in 12 different CI jobs (the link just goes to one of them).

Generated by Testclutch

@jay

jay commented Jul 9, 2026

Copy link
Copy Markdown
Member

If retrieving the terminal size via stdin is a problem because of redirect, then that could also be true of stdout, stderr etc, so should there be a isatty check or something like that (edit: I think the ioctl is probably sufficient if it checks that)

@jay

jay commented Jul 9, 2026

Copy link
Copy Markdown
Member

also why does it say /* You are not expected to understand this */ and there's an extra guard that makes no sense

Comment thread src/terminal.c Outdated
Comment thread src/terminal.c Outdated
@jay jay added the needs-info label Jul 15, 2026
@jay

jay commented Jul 15, 2026

Copy link
Copy Markdown
Member

The reporter seems to be MIA on this but I think it's probably a good idea to get terminal columns from stderr , stdout etc if stdin isn't a terminal. @bagder is there a reason you chose stdin, did it seem most likely, or do you have any objection to this change?

@bagder bagder left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, assuming the minor nits are addressed.

Comment thread src/terminal.c Outdated
@bagder

bagder commented Jul 16, 2026

Copy link
Copy Markdown
Member

is there a reason you chose stdin

Sorry, I can't recall why I selected stdin for this!

@jay jay removed the needs-info label Jul 24, 2026
- Get the terminal size from STDOUT or STDERR when the terminal size of
  STDIN is not available.

Closes curl#22276
@jay jay closed this in 20c7877 Jul 24, 2026
@jay
jay merged commit 20c7877 into curl:master Jul 24, 2026
213 checks passed
@jay

jay commented Jul 24, 2026

Copy link
Copy Markdown
Member

Despite no response from the author I've landed this with everyone's suggested changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants