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

ioctl system call returning the wrong result #7375

Open
2 tasks done
sxyazi opened this issue Jul 30, 2023 · 6 comments
Open
2 tasks done

ioctl system call returning the wrong result #7375

sxyazi opened this issue Jul 30, 2023 · 6 comments

Comments

@sxyazi
Copy link

sxyazi commented Jul 30, 2023

  • Your Hyper.app version is 3.4.1. Please verify you're using the latest Hyper.app version
  • I have searched the issues of this repo and believe that this is not a duplicate

  • Any relevant information from devtools? (CMD+OPTION+I on macOS, CTRL+SHIFT+I elsewhere):

No

  • Is the issue reproducible in vanilla Hyper.app?

Yes

Issue

Can't use ioctl to get the correct value of xpixel and ypixel:

let s = unsafe {
	let s: winsize = std::mem::zeroed();
	ioctl(STDOUT_FILENO, TIOCGWINSZ, &s);
	s
};
dbg!(s.ws_col, s.ws_row, s.ws_xpixel, s.ws_ypixel);
image

There are both zero, which causes some apps don't work.


  • Hyper version: undefined "3.4.1"
  • OS ARCH VERSION: darwin arm64 22.5.0
  • Electron: 20.3.6 LANG: undefined
  • SHELL: /opt/homebrew/bin/zsh TERM: undefined
.hyper.js contents
{
  "updateChannel": "stable",
  "fontSize": 12,
  "fontFamily": "Menlo, \"DejaVu Sans Mono\", Consolas, \"Lucida Console\", monospace",
  "fontWeight": "normal",
  "fontWeightBold": "bold",
  "lineHeight": 1,
  "letterSpacing": 0,
  "cursorColor": "rgba(248,28,229,0.8)",
  "cursorAccentColor": "#000",
  "cursorShape": "BLOCK",
  "cursorBlink": false,
  "foregroundColor": "#fff",
  "backgroundColor": "#000",
  "selectionColor": "rgba(248,28,229,0.3)",
  "borderColor": "#333",
  "css": "",
  "termCSS": "",
  "workingDirectory": "",
  "showHamburgerMenu": "",
  "showWindowControls": "",
  "padding": "12px 14px",
  "colors": {
    "black": "#000000",
    "red": "#C51E14",
    "green": "#1DC121",
    "yellow": "#C7C329",
    "blue": "#0A2FC4",
    "magenta": "#C839C5",
    "cyan": "#20C5C6",
    "white": "#C7C7C7",
    "lightBlack": "#686868",
    "lightRed": "#FD6F6B",
    "lightGreen": "#67F86F",
    "lightYellow": "#FFFA72",
    "lightBlue": "#6A76FB",
    "lightMagenta": "#FD7CFC",
    "lightCyan": "#68FDFE",
    "lightWhite": "#FFFFFF",
    "limeGreen": "#32CD32",
    "lightCoral": "#F08080"
  },
  "shell": "",
  "shellArgs": [
    "--login"
  ],
  "env": {},
  "bell": "SOUND",
  "copyOnSelect": false,
  "defaultSSHApp": true,
  "quickEdit": false,
  "macOptionSelectionMode": "vertical",
  "webGLRenderer": true,
  "webLinksActivationKey": "",
  "disableLigatures": true,
  "disableAutoUpdates": false,
  "screenReaderMode": false,
  "preserveCWD": true
}
plugins
{
  "plugins": [],
  "localPlugins": []
}
@jerch
Copy link

jerch commented Aug 4, 2023

There are both zero, which causes some apps don't work.

Apps must not rely on correct pixel propagation by TIOCGWINSZ. These values are intended to be 0 as a fallback Even worse - there are systems, that cannot tell a pixel size at all.

So its not returning a wrong result here, but telling you - "dont know the right value". Would be nice to see this getting fixed yes, but imho thats an issue with node-pty.

@sxyazi
Copy link
Author

sxyazi commented Aug 5, 2023

there are systems, that cannot tell a pixel size at all

Is it a non-Unix system, like Windows?

I'm developing a terminal file manager, and when I test its image preview feature, I find that I can't run it in Hyper. I need to get the actual pixel size to calculate the image's width and height -- I'm not sure how else can get it done.

@jerch
Copy link

jerch commented Aug 5, 2023

You can also use CSI 14 t to request the current text area size in pixels.

@sxyazi
Copy link
Author

sxyazi commented Aug 7, 2023

After testing, Hyper doesn't even support CSI 14 t:

image

But VSCode supports it -- which also uses node-pty and does not fully implement TIOCGWINSZ:

image

@jerch
Copy link

jerch commented Aug 7, 2023

Are you on latest canary? CSI 14 t gets activated by the recently added image support and prolly will not work with older versions.

@sxyazi
Copy link
Author

sxyazi commented Aug 7, 2023

I did a quick test on the latest canary, it supports CSI 14 t now!

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

No branches or pull requests

2 participants