Screen margins and scrolling regions are acting strange for the JOE editor #1816

@DHowett-MSFT commented on GitHub (Jun 24, 2019):

This may have something to do with scrolling regions.

@DHowett-MSFT commented on GitHub (Jun 24, 2019): This may have something to do with scrolling regions.

claunia commented

@stobor827 commented on GitHub (Jun 29, 2019):

Very interested in this. I believe joe is not curses-based, so that might explain diff from other editors.
This also happens in the old WSL terminal. (or via ps and ssh). Not positive it always did, but at least in 1803 and 1809.
PageUp/Dn is also broken.
Line/screen refreshes due to paste inserts and some editing actions broken too.

Are there docs on how to sniff/debug the control characters? I would be interested in helping to root cause or find smaller repro steps, but don't know where to start.

@stobor827 commented on GitHub (Jun 29, 2019): Very interested in this. I believe joe is not curses-based, so that might explain diff from other editors. This also happens in the old WSL terminal. (or via ps and ssh). Not positive it always did, but at least in 1803 and 1809. PageUp/Dn is also broken. Line/screen refreshes due to paste inserts and some editing actions broken too. Are there docs on how to sniff/debug the control characters? I would be interested in helping to root cause or find smaller repro steps, but don't know where to start.

claunia commented

@j4james commented on GitHub (Jun 30, 2019):

I think this is related to issue #141, or at least was probably caused by attempts to fix that issue. For a simple test case, you can enter the following commands in a bash shell:

ls -la && printf "\e[r\n\e[100M"

The ls -la is just to fill the screen with some content. The printf is generating the escape sequences to clear the margins, move down one line, and then delete 100 lines from the screen. You'll notice that it doesn't actually delete anything though.

I think the same thing is happening in joe when you scroll - it's moving to the second line of the screen (below the status bar) and then deleting a line to force the rest of the screen to scroll up. Since the delete doesn't work, the content doesn't scroll, and only the last line is updated.

The reason for the failure is there is a check in the DoSrvPrivateModifyLinesImpl function to make sure the cursor is within the bounds of the margins, but this fails when the margins are set to the full screen (that is a special case that needs to be tested separately).

This is essentially a one line fix, but I'd be happy to submit a PR for it if that would help.

@j4james commented on GitHub (Jun 30, 2019): I think this is related to issue #141, or at least was probably caused by attempts to fix that issue. For a simple test case, you can enter the following commands in a bash shell: ls -la && printf "\e[r\n\e[100M" The `ls -la` is just to fill the screen with some content. The `printf` is generating the escape sequences to clear the margins, move down one line, and then delete 100 lines from the screen. You'll notice that it doesn't actually delete anything though. I think the same thing is happening in _joe_ when you scroll - it's moving to the second line of the screen (below the status bar) and then deleting a line to force the rest of the screen to scroll up. Since the delete doesn't work, the content doesn't scroll, and only the last line is updated. The reason for the failure is there is a check in the `DoSrvPrivateModifyLinesImpl` function to make sure the cursor is within the bounds of the margins, but this fails when the margins are set to the full screen (that is a special case that needs to be tested separately). This is essentially a one line fix, but I'd be happy to submit a PR for it if that would help.

claunia commented

@j4james commented on GitHub (Jun 30, 2019):

Having a run a few of my old margin tests, I seen now it's actually a little more complicated than I originally thought. The delete doesn't just fail when the margin is fullscreen - it'll also fail any time the cursor isn't in the first column.

This is again related to the bounds check in DoSrvPrivateModifyLinesImpl. It's using the Viewport::IsInBounds method to check whether the cursor is inside the margins, but the left and right margins will always be set to 0, so that'll only work on the first column of the screen.

This is likely an issue in other places too. At the very least I can see IsInBounds being used with margins in the DoSrvPrivateReverseLineFeed method, which will cause the RI sequence (ESC M) to fail in similar situations.

@j4james commented on GitHub (Jun 30, 2019): Having a run a few of my old margin tests, I seen now it's actually a little more complicated than I originally thought. The delete doesn't just fail when the margin is fullscreen - it'll also fail any time the cursor isn't in the first column. This is again related to the bounds check in `DoSrvPrivateModifyLinesImpl`. It's using the `Viewport::IsInBounds` method to check whether the cursor is inside the margins, but the left and right margins will always be set to 0, so that'll only work on the first column of the screen. This is likely an issue in other places too. At the very least I can see `IsInBounds` being used with margins in the `DoSrvPrivateReverseLineFeed` method, which will cause the RI sequence (`ESC M`) to fail in similar situations.

claunia commented

@DHowett-MSFT commented on GitHub (Jun 30, 2019):

Tried to rename this to adequately capture more of what's happening. 😄 Thanks for digging into all our VT stuff, @j4james!

@DHowett-MSFT commented on GitHub (Jun 30, 2019): Tried to rename this to adequately capture more of what's happening. :smile: Thanks for digging into all our VT stuff, @j4james!

claunia commented

@j4james commented on GitHub (Jul 3, 2019):

Btw, I'd be happy to create a PR for this if nobody else is working on it.

As a minimal fix, I would just update the DoSrvPrivateReverseLineFeed and DoSrvPrivateModifyLinesImpl methods to handle the bounds check correctly and make sure they're both handing the case where the margins aren't set.

However, it might make things a little more readable if we could refactor the bounds checking code into a new IsCursorInMargins method in the SCREEN_INFORMATION class, and make better use of the existing AreMarginsSet method. This would mean also updating the DoSrvMoveCursorVertically and AdjustCursorPosition methods, both of which perform similar margin tests.

That refactoring might come with a price, though. In some cases it could be a little more efficient (typically when the margins aren't set, so it can short circuit other checks), but in other cases it might be less efficient (because the absolute margin boundaries would sometimes need to be calculated in more than one place). It might not be that big a deal if the compiler can optimize it, though.

Anyway, if you'd like me to take this on, and you have a preference regarding the implementation, just let me know.

@j4james commented on GitHub (Jul 3, 2019): Btw, I'd be happy to create a PR for this if nobody else is working on it. As a minimal fix, I would just update the `DoSrvPrivateReverseLineFeed` and `DoSrvPrivateModifyLinesImpl` methods to handle the bounds check correctly and make sure they're both handing the case where the margins aren't set. However, it might make things a little more readable if we could refactor the bounds checking code into a new `IsCursorInMargins` method in the `SCREEN_INFORMATION` class, and make better use of the existing `AreMarginsSet` method. This would mean also updating the `DoSrvMoveCursorVertically` and `AdjustCursorPosition` methods, both of which perform similar margin tests. That refactoring might come with a price, though. In some cases it could be a little more efficient (typically when the margins aren't set, so it can short circuit other checks), but in other cases it might be less efficient (because the absolute margin boundaries would sometimes need to be calculated in more than one place). It might not be that big a deal if the compiler can optimize it, though. Anyway, if you'd like me to take this on, and you have a preference regarding the implementation, just let me know.

claunia commented

@zadjii-msft commented on GitHub (Jul 3, 2019):

@j4james You definitely seem to have a handle on the situation, go for it!

Your description of the refactoring work is definitely something I've thought about before but never had the time to get to, so I'd say try it out.

Scrolling with the margins is something that's been a perpetual bug factory, so more unittests would certainly be appreciated here (most of the current ones are in ScreenBufferTests, but I don't think I need to tell you that 😝)

@zadjii-msft commented on GitHub (Jul 3, 2019): @j4james You definitely seem to have a handle on the situation, go for it! Your description of the refactoring work is definitely something I've thought about before but never had the time to get to, so I'd say try it out. Scrolling with the margins is something that's been a perpetual bug factory, so more unittests would certainly be appreciated here (most of the current ones are in ScreenBufferTests, but I don't think I need to tell you that 😝)

claunia commented

@curry684 commented on GitHub (Feb 18, 2020):

I'm still experiencing these issues today on Win10 1909 (18363.592), while it is supposedly fixed in that version. Driving me mad as 20 year Joe lover doing most of his Linux sysadmin jobs in Ubuntu for Windows 😉

Is there a component I forgot to update?

@curry684 commented on GitHub (Feb 18, 2020): I'm still experiencing these issues today on Win10 1909 (18363.592), while it is supposedly fixed in that version. Driving me mad as 20 year Joe lover doing most of his Linux sysadmin jobs in Ubuntu for Windows 😉 Is there a component I forgot to update?

claunia commented

@DHowett-MSFT commented on GitHub (Feb 18, 2020):

1909 isn’t the version that has this fix.

@DHowett-MSFT commented on GitHub (Feb 18, 2020): 1909 isn’t the version that has this fix.

claunia commented

@curry684 commented on GitHub (Feb 19, 2020):

@DHowett-MSFT thanks for the swift response. Apart from switching my entire Windows to Insider (which I'd prefer not to) is there any way to get this fix on my computer?

@curry684 commented on GitHub (Feb 19, 2020): @DHowett-MSFT thanks for the swift response. Apart from switching my entire Windows to Insider (which I'd prefer not to) is there any way to get this fix on my computer?

claunia commented

@DHowett-MSFT commented on GitHub (Feb 20, 2020):

@curry684 lets put it this way: i technically can’t stop you from building a release build of Host.EXE (OpenConsole) from this repository and replacing conhost.exe on your machine with it. 😉

@DHowett-MSFT commented on GitHub (Feb 20, 2020): @curry684 lets put it this way: _i technically can’t stop you_ from building a release build of Host.EXE (OpenConsole) from this repository and replacing conhost.exe on your machine with it. 😉

claunia commented

@gunterzielke commented on GitHub (Mar 13, 2020):

I also just ran into this joe problem and wonder if there is any information (or guess) when this fix will make it into the regular release. I am running 1909 and don't want to climb up the learning curve to - unofficially - build the host.exe myself :-)

@gunterzielke commented on GitHub (Mar 13, 2020): I also just ran into this joe problem and wonder if there is any information (or guess) when this fix will make it into the regular release. I am running 1909 and don't want to climb up the learning curve to - unofficially - build the host.exe myself :-)

claunia commented

@DHowett-MSFT commented on GitHub (Mar 13, 2020):

@gunterzielke in general, a console host bug that's fixed will be available to the general public starting with a windows release. Since this one wasn't in 1909, it may be in the one that comes after 1909. 😄

@DHowett-MSFT commented on GitHub (Mar 13, 2020): @gunterzielke in general, a console host bug that's fixed will be available to the general public starting with a windows release. Since this one wasn't in 1909, it may be in the one that comes after 1909. :smile:

claunia commented

@gunterzielke commented on GitHub (Mar 13, 2020):

Oh well - I did ask for a guess, didn't I :-)
For the time being, I will then get used to nano.
Maybe I will like this better, or maybe even emacs (the stuff for real men who don't each quiche).

@gunterzielke commented on GitHub (Mar 13, 2020): Oh well - I did ask for a guess, didn't I :-) For the time being, I will then get used to nano. Maybe I will like this better, or maybe even emacs (the stuff for real men who don't each quiche).

claunia commented

2026-01-30 22:37:23 +00:00

@curry684 commented on GitHub (Mar 13, 2020):

Just a few weeks left until 20H1 😉

Apart from that - I find it weird that Microsoft still has issues like this in treating a huge 'distro' like a monolith. In just about every other OS if a subsystem or component has an issue you can do something like apt-get upgrade 3 hours later to get a fix for just that component.

Why is Microsoft still withholding bug fixes for over 6 months like it's still 2002?

@curry684 commented on GitHub (Mar 13, 2020): Just a few weeks left until 20H1 :wink: Apart from that - I find it weird that Microsoft still has issues like this in treating a huge 'distro' like a monolith. In just about every other OS if a subsystem or component has an issue you can do something like `apt-get upgrade` 3 hours later to get a fix for *just that component*. Why is Microsoft still withholding bug fixes for over 6 months like it's still 2002?

claunia commented

@DHowett-MSFT commented on GitHub (Mar 13, 2020):

@curry684 I don't disagree with you ;)

Incidentally, that's why Terminal is not shipping with the operating system. It actually contains its own separate undocked copy of the console host so it isn't subject to the 6-month release cycle of our platform components. 😄

@DHowett-MSFT commented on GitHub (Mar 13, 2020): @curry684 I don't disagree with you ;) Incidentally, that's why Terminal is not shipping with the operating system. It actually contains its own separate undocked copy of the console host so it isn't subject to the 6-month release cycle of our platform components. :smile:

claunia commented

2026-01-30 22:37:23 +00:00

@uiteoi commented on GitHub (Dec 14, 2020):

Also running version 1909 on a computer bought in August. Please fix this ASAP, I rely on Joe for a lot of work. Thanks

@uiteoi commented on GitHub (Dec 14, 2020): Also running version 1909 on a computer bought in August. Please fix this ASAP, I rely on Joe for a lot of work. Thanks

claunia commented

2026-01-30 22:37:23 +00:00

@curry684 commented on GitHub (Dec 14, 2020):

Then why don't you upgrade to 20H1 or 20H2, both contain the fix, that's the point of updating....

@curry684 commented on GitHub (Dec 14, 2020): Then why don't you upgrade to 20H1 or 20H2, both contain the fix, that's the point of updating....

claunia commented

@zadjii-msft commented on GitHub (Dec 14, 2020):

Yea, this was fixed almost 18 months ago. I'm not sure I can fix it any sooner than that 😆

@zadjii-msft commented on GitHub (Dec 14, 2020): Yea, this was fixed almost 18 months ago. I'm not sure I can fix it any sooner than that 😆

claunia commented

@curry684 commented on GitHub (Dec 14, 2020):

Also, as @DHowett-MSFT hinted Windows Terminal is a brilliant product and not affected by this issue, and it has an incredible support cycle. Once I switched over a few months ago I never looked back, used to have Ubuntu on VMware, now 100% WSL2 and haven't looked back since.

@curry684 commented on GitHub (Dec 14, 2020): Also, as @DHowett-MSFT hinted [Windows Terminal](https://www.microsoft.com/nl-nl/p/windows-terminal/) is a brilliant product and not affected by this issue, and it has an incredible support cycle. Once I switched over a few months ago I never looked back, used to have Ubuntu on VMware, now 100% WSL2 and haven't looked back since.

claunia commented

@uiteoi commented on GitHub (Dec 15, 2020):

Then why don't you upgrade to 20H1 or 20H2, both contain the fix, that's the point of updating....

I didn't know there was an available update that we could force manualy. Doing it right now. Thanks for tip.

@uiteoi commented on GitHub (Dec 15, 2020): > Then why don't you upgrade to 20H1 or 20H2, both contain the fix, that's the point of updating.... I didn't know there was an available update that we could force manualy. Doing it right now. Thanks for tip.

claunia commented