Windows: fix iread

[adoxa]

The Windows version of iread removed the character, causing an extended sequence to be incorrectly split. Add an unget function to put the character back. Resolves #378.

[avih]

Thanks. I can confirm it fixes #378 for me.

But I don't think it would work for unicode chars, because the unicode path at WIN32getch checks utf8_current_byte < utf8_size before it looks at currentKey.unicode.

Also note that #381 wants to change a bit how unicode is handled.

[adoxa]

I don't think Unicode'll be an issue, since it's really only the combination of page down during reading the file that has the problem. With that in mind, the new Unicode code won't be affected.

[avih]

I don't think Unicode'll be an issue, since it's really only the combination of page down during reading the file that has the problem

Yes, this fixes the issue where it needs to unget the 0 which was the "prefix" of a scan code.

However, it doesn't necessarily work correctly in all other cases.

For instance, it unconditionally increases keyCount, so cases where WIN32getch didn't decrease it will not be restored to exactly the same state as before.

For instance

• Any char of a UTF-8 sequence except the first.
• \003 in case of ABORT_SIGS().

Another issue: after returning the second part of a scancode X (not 0): WIN32ungetch does pending_scancode = 0, so the next call to WIN32getch will return 0 instead of X.

The scan code issue is new since 6fd6739 , because it relied on a buggy behavior of the unicode code (return UTF8 sequence if ascii != unicode, and WIN32ungetc changes only unicode, so the buggy code was entered), which was fixed in that commit.

There are also other issues.

The main underlaying issue is that the state is non trivial and composed of many variables: x11mouse state, scan code state, utf8 state, each with its own variables and buffers.

So at the very least it's very cumbersome to tell exactly what the previous state was by looking at the current state and the last return value from WIN32getch, and it's most probably not worth trying. In the case of UTF-8 is strictly impossible without bigger scope changes, because the UTF-8 state is currently invisible to WIN32ungetch

It's OK for now because it can successfully unget the 0-prefix of a scan code, which was the main trigger to add WIN32ungetch, but I intend to submit a PR to make the keyboard state much simpler, and consequently make WIN32ungetch work correctly in all cases.

So just an FYI.

[avih]

I've submitted #388 which should make WIN32ungetch work on 100% of the cases.

I opted for a much smaller fix, without refactoring the KB queue system.

If anyone is interested, a proof of concept patch which unifies the KB queues and also allows a trivial WIN32ungetch is available at the top commit in this branch https://github.com/avih/less/commits/winkbq

It was written before WIN32ungetch was added, so it's not implemented, but it would be:

public void WIN32ungetch(int ch)
{
	if (kbq_free())
		kbq_ungetc(c);
}

I don't intend to open a PR for it because the KB queue system does work for the most part, even if the state can be non trivial, but I can open a PR if @gwsw shows explicit interest.