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

Fix the detection of the current terminal size #6244

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

kit-ty-kate
Copy link
Member

@kit-ty-kate kit-ty-kate commented Oct 16, 2024

Fixes #6243

Tested on:

  • Linux/glibc
  • Linux/musl
  • macOS
  • FreeBSD
  • OpenBSD
  • NetBSD
  • Cygwin

Can't test:

  • DragonFlyBSD (they package manager just crashes after an update), however given DragonFlyBSD derives from FreeBSD, there is no doubt to guess it would work just the same.
  • MSYS2 (i couldn't find the way to build an opam targeting msys2 instead of Windows, but like DragonFlyBSD, MSYS2 is a fork of Cygwin so it should just work)

The only change of behaviour between the code previous to #6230 and this one is that prior it would default to 80 column only if you had all 3 (stdout, stdin, stderr) redirected, but now it would default to it if only stdout is redirected, which i think is an improvement.

Backported to 2.3 in #6246

Copy link
Collaborator

@rjbou rjbou left a comment

Choose a reason for hiding this comment

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

I think it worth for this fix to add in the commit the os tested, like that we have a snapshot of where it is working when added.
Except the API changes, lgtm!


let that's_a_no_no _ = failwith "Unix only. This function isn't implemented."

let get_stdout_ws_col = that's_a_no_no
Copy link
Collaborator

Choose a reason for hiding this comment

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

the first Unix stub :D

master_changes.md Show resolved Hide resolved
@kit-ty-kate
Copy link
Member Author

requested change done

Tested on:
* Linux/glibc
* Linux/musl
* macOS
* FreeBSD
* OpenBSD
* NetBSD
* Cygwin
@kit-ty-kate
Copy link
Member Author

rebased

Copy link
Collaborator

@rjbou rjbou left a comment

Choose a reason for hiding this comment

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

Thanks!

@kit-ty-kate kit-ty-kate merged commit 126960e into ocaml:master Oct 21, 2024
40 checks passed
@kit-ty-kate kit-ty-kate deleted the ioctl_winsz branch October 21, 2024 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[2.3.0~beta1 regression] Long columns are misaligned
2 participants