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

[JJussi]

Environment

Windows build number:Microsoft Windows [Version 10.0.18922.1000]
Windows Terminal version (if applicable):wsl2

Steps to reproduce

wsl
sudo apt install joe
joe some_text_file.txt

Expected behavior

At windows subsystem linux terminal you start editing some text file with joe editor.
When file have more lines that what current terminal have and you scroll down/up, lines at the screen scrolls with you.

This works fine with nano/pico/vi but not with joe.

Actual behavior

When cursor reach last/first line of screen, only last/first line changes under cursor. Screen is not scrolling.

[DHowett-MSFT]

This may have something to do with scrolling regions.

[stobor827]

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.

[j4james]

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]

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.

[DHowett-MSFT]

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

[j4james]

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.

[zadjii-msft]

@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 😝)

[curry684]

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?

[DHowett-MSFT]

1909 isn’t the version that has this fix.

[curry684]

@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?

[DHowett-MSFT]

@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. 😉

[gunterzielke]

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 :-)

[DHowett-MSFT]

@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. 😄

[gunterzielke]

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).

[curry684]

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?

[DHowett-MSFT]

@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. 😄

[uiteoi]

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

[curry684]

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

[zadjii-msft]

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

[curry684]

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.

[uiteoi]

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.

[curry684]

For future reference: https://docs.microsoft.com/en-us/windows/release-information/