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 docs for window_size #841

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

benjajaja
Copy link
Contributor

@benjajaja benjajaja commented Nov 4, 2023

  1. Fix external link formatting.
  2. Be specific about all fields possibly being zero, and the pixel fields being documented for linux as "unused".
  3. (I am not sure about TIOCGWINSZ bing posix).
  4. Specify that window_size() always returns an error on windows.
  5. Put the detailed docs in WindowSize.

@markus-bauer this addresses some points of your comment in #790 after it was merged. What do you suggest regarding windows not being supported? I have tried everything in that regard:

  1. Getting the console font size, but it is in "logical units", so it would need the current DPI or something like that to get to the font pixel size, to ultimately get the window size by multiplying with (columns,rows). I doubt it would be precise or reliable.
  2. All of the windows console API only deals in (columns,rows).
  3. Using GetClientRect, which I almost thought would work, but even that gave me bogus values. Also seems a bit far off the track for a terminal library.

Perhaps this was a mistake, and window_size() should be removed from crossterm, after all it's not cross-platform now. Users who need the window size, on unix, can then just use some tty-ioctl lib, e.g. rustix.

My motivation was to make a library that can render images in the TUI apps, supporting sixels, kitty-graphics, or even ascii as fallback. It's necessary to know a character "cell" size in pixels to correctly up- or downscale an image into a given (columns,rows) area.

1. Fix external link formatting.
2. Be specific about all fields possibly being zero, and the pixel
   fields being documented for **linux** as "unused".
3. (I am not sure about TIOCGWINSZ bing posix).
4. Specify that `window_size()` always returns an error on windows.
5. Put the detailed docs in `WindowSize`.
@benjajaja benjajaja requested a review from TimonPost as a code owner November 4, 2023 09:56
@markus-bauer
Copy link

markus-bauer commented Nov 4, 2023

It's cool of you to spend additional time on this. The new documentation is better and links seem to work now.

Since @TimonPost merged it, I assume he's okay with adding it. I already said in the old PR, that there is probably no common solution and if you want it for unix, it's trivial to use rustix.
But it doesn't matter as long as the implementation and documentations is correct.

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.

3 participants