microsoft / terminal

The new Windows Terminal and the original Windows console host, all in the same place!
MIT License
95.12k stars 8.25k forks source link

The Resize Window escape sequence doesn't support omitted and zero parameters #4417

Open j4james opened 4 years ago

j4james commented 4 years ago

Environment

Windows build number: Version 10.0.18362.535

Steps to reproduce

  1. Open a bash shell in conshot
  2. Set the window size to 80x25 with printf "\e[8;25;80t"
  3. Set just the height parameter to 30 with printf "\e[8;30t"
  4. Set just the width parameter to 100 with printf "\e[8;;100t"
  5. Set both width and height to zero with printf "\e[8;0;0t"

Expected behavior

According to the XTerm documentation, omitted parameters should reuse the current height or width, while zero parameters should use the display's height or width.

Actual behavior

Step 2 works as expected, but the remaining steps have no effect.

That said, this functionality doesn't seem to be widely supported (the only terminals I've seen that got it right were XTerm and Mintty), so it's probably not that important.

egmontkob commented 4 years ago

omitted parameters should reuse the current height or width, while zero parameters should use the display's height or width

My interpretation of ECMA-48 5.4.2

e) An empty parameter sub-string represents a default value which depends on the control function

is that an empty parameter must be equivalent to a certain valid explicitly specified value. Given that the values in this context are expected to be unsigned integers, the default must also be an unsigned int, like 0, 1, 999 or so – leaving room for only one special action for the number 0, and not being able to express two different special actions ("unchanged" vs. "maximized"). I might be wrong, though.

On a more important note, I believe that this escape sequence with a fixed size is borderline problematic/harmful, whereas this one with the "maximized" value, and other nearby escape sequences, are straight harmful (just like when webpages that are allowed to move/resize the browser window – everyone hates that, right?) and terminals should deliberately refuse to implement. (Yeah I know vttest tests for 80/132 column switching...) See also https://gitlab.gnome.org/GNOME/vte/issues/128#note_524609.

j4james commented 4 years ago

My interpretation of ECMA-48 5.4.2

e) An empty parameter sub-string represents a default value which depends on the control function

is that an empty parameter must be equivalent to a certain valid explicitly specified value.

I think you're reading too much into that. At any rate, this sort of thing isn't without precedent. The second parameter of DECSTBM has a default value of "current number of lines per screen" (although in that case, zero is at least the same as the default).

Still, like it or not, that's how the operation is defined. Whether you choose to support it or not is a separate question, and not one I expect everyone to agree on. We already do support it (at least in conhost), so this issue is just about getting us more closely aligned with the specification.

On a more important note, I believe that this escape sequence with a fixed size is borderline problematic/harmful, whereas this one with the "maximized" value, and other nearby escape sequences, are straight harmful

Personally I don't have a problem with apps resizing the window, because a regular Windows console app would already be able to do that. And I wouldn't want to ban all apps from using certain functionality just because some trollish app might decide to do something annoying with that functionality - a resized window isn't going to kill me. And if that is a major concern for users, we could always add an option to disable sizing ops (I think XTerm has something like that).

DHowett-MSFT commented 4 years ago

From the perspective of "conhost supports this, so WT should support this" I totally agree.

Alternatively, I rather dislike how this works in a world where multiple terminals might be hosted in one application window. This will be uncomfortable to implement with split panes as well. :smile:

DHowett-MSFT commented 4 years ago

(I do think that conhost should do something with 0, so I've tagged this up mostly for conhost's sake. What that thing is should be determined by what most other terminal emulators do.)

j4james commented 4 years ago

I was only expecting this to work in conhost. WT's handling of programmatic resizing is completely broken, and I'm not sure if there is any way to fix it. At any rate, I think it's a big enough problem that it should probably be dealt with as a separate issue.

DHowett-MSFT commented 4 years ago

Excellent. Re-tagged.