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

[Enhancement] Resize tiled windows by px instead of ppt #3239

Closed
andrewla opened this Issue Apr 13, 2018 · 5 comments

Comments

Projects
None yet
4 participants
@andrewla

andrewla commented Apr 13, 2018

I've found myself frustrated by the inability to exactly resize windows in tiles, especially when I'm resizing a terminal that I want to get to a specific text width. With the mouse, I can do pixel-perfect resizing, but otherwise I'm stuck with ppt, and the smallest percentage point is 1, since floats aren't supported, so there's effectively no way to resize with the keyboard at that level of precision.

I have a PR almost ready with this feature; I'm ironing out the kinks now and working on test cases. I've been running it on my development workstation for a while now.

The old docs say:

The optional pixel argument specifies by how many pixels a floating container should be grown or shrunk (the default is 10 pixels). The ppt argument means percentage points and specifies by how many percentage points a tiling container should be grown or shrunk (the default is 10 percentage points).

And I would update them to say:

The optional pixel argument specifies by how many pixels a container should be grown or shrunk (the default is 10 pixels). The optional ppt argument means "percentage points", and if specified it indicates that a tiling container should be grown or shrunk by that many points, instead of by the px value.

If a ppt value is specified, it will be preferred to the px value for tiling containers only. So this change is somewhat backwards compatible, in that configurations that specify both px and ppt would continue to behave the same way.

An alternative solution would be to allow fractional ppt value (I had a PR for this ready as well, but it was uglier and harder to control).

@i3bot i3bot added the enhancement label Apr 13, 2018

@Airblader

This comment has been minimized.

Member

Airblader commented Apr 13, 2018

Doesn't #3036 cover your case already?

@andrewla

This comment has been minimized.

andrewla commented Apr 13, 2018

#3036 still uses ppt instead of px, so it prevents me from doing an exact resize without doing the math externally, or by repeatedly experimenting.

@Airblader

This comment has been minimized.

Member

Airblader commented Apr 13, 2018

I see. You said your change is "somewhat" backwards compatible — where does it fail to be compatible? Also, given that you worked on this already I think it might make sense to discuss it over the PR to see what it'd look like.

@andrewla

This comment has been minimized.

andrewla commented Apr 13, 2018

Okay, but I don't have any testcases and have not yet run the test suite against the change, but with those caveats I'll start the PR.

By "somewhat" I mean that if a configuration specifies only a px in a resize, then the px will now be used instead of defaulting to 10ppt. (EDIT: I mis-wrote this as being the other way around)

@Airblader

This comment has been minimized.

Member

Airblader commented Apr 14, 2018

I don't have any testcases

That's fine, PRs can be updated. :-) We generally recommend first discussing, then implementing in case the feature is not accepted. Since you have written the code already, however, there's no reason not to look at it as well.

have not yet run the test suite against the change

Travis will do this for you now, too. ;-)

By "somewhat" I mean that if a configuration specifies only a px in a resize, then the px will now be used instead of defaulting to 10ppt.

I think that's an acceptable »breaking« change.

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