Skip to content
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

Add support for the HPA escape sequence #3348

Closed
j4james opened this issue Oct 27, 2019 · 4 comments · Fixed by #3368
Closed

Add support for the HPA escape sequence #3348

j4james opened this issue Oct 27, 2019 · 4 comments · Fixed by #3368
Labels
Area-VT Virtual Terminal sequence support Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Conhost For issues in the Console codebase Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@j4james
Copy link
Collaborator

j4james commented Oct 27, 2019

Description of the new feature/enhancement

HPA is an escape sequence for setting the horizontal cursor position.

Technically there is a distinction between HPA, which sets the data position, and CHA, which sets the presentation position (at least according to ECMA-048). However, I believe that only applies to bidirectional environments, and from what I've seen, it looks like most terminal emulators just treat them as synonyms.

Since we already support CHA, this doesn't really provide us with any new functionality, but it is one of the sequences tested by Vttest, so it would be nice to support, and it should be trivial to add.

Proposed technical implementation details (optional)

  1. Add a new entry in the VTActionCodes enum.
  2. Update the OutputStateMachineEngine::ActionCsiDispatch method to treat that new code as a synonym for the existing CHA_CursorHorizontalAbsolute.

If you care about tracking it as a separate telemetry entry, that would require a little more duplication of code, but it's still fairly trivial.

@j4james j4james added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Oct 27, 2019
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Oct 27, 2019
@egmontkob
Copy link

egmontkob commented Oct 27, 2019

However, I believe that only applies to bidirectional environments

Yes, this distinction is due to BiDi. ECMA's BiDi was designed way before the Unicode BiDi Algorithm, with apparently hardly any experience or prior art in the topic. There's no known implementation of ECMA 48 and TR/53's BiDi support I could find being even mentioned. As I detail in my revamped BiDi proposal, ECMA suffers from quite a few problems, including that the idea of doing any operation on the presentational component is just fundamentally broken.

So, using HPA would be the proper choice, as opposed to CHA which is used in practice.

Similarly, HPB and HPR instead of CUB and CUF. Interestingly, HPB is missing from VT510 (according to the doc you linked) and xterm doesn't implement it either.

None of these HP_s are implemented by VTE. VTE only implements HPA out of the HP_ ones.

I guess we'll just have to accept that CHA, CUB, CUF have become what HPA, HPB, HPR were designed to be, and it's unlikely to ever change. The fix would be to revise the documentation around the data vs. presentation components, and as a by-product of that, redefine CHA, CUB, CUF to operate on the data component (the only place where it makes sense to operate).

tested by Vttest

In which submenu exactly?

Let's note that the documentation you linked is obviously buggy, it says for HPA and HPR:

Inquire as to the amount of free memory for programmable key operations.

which is, like, what the heck?

@j4james
Copy link
Collaborator Author

j4james commented Oct 28, 2019

Similarly, HPB and HPR instead of CUB and CUF.

I was looking at those too (as well as VPB and VPR), but I didn't think they were exact synonyms for the CUx commands - I think there's a difference in the margin handling. I'll raise a separate issue for them once I have a better idea of how they're supposed to function (at least HPR and VPR - I don't really care about HPB and VPB).

tested by Vttest

In which submenu exactly?

Under Test non-VT100 there's a ISO-6429 cursor-movement section which includes the HPA test, and then there's also a VT520 section, which has a Cursor-movement subsection, which includes the HPA test.

Let's note that the documentation you linked is obviously buggy, it says for HPA and HPR:

Inquire as to the amount of free memory for programmable key operations.

Yeah I noticed that. There are a bunch of commands with that mistake - it's in the original PDF of the VT510 manual too. The mistake was fixed in the VT520/VT525 manual, but there's no HTML version of that, so it's not great for linking.

@egmontkob
Copy link

Whoops, I was wrong about VTE: It does support HPA and VPA, but not the rest.

as well as VPB and VPR), but I didn't think they were exact synonyms for the CUx commands - I think there's a difference in the margin handling

I'll double check VTE when you open that other issue with your margin handling findings (the VP_ ones ignoring the scrolling region?).

Thanks!

@zadjii-msft zadjii-msft added Area-VT Virtual Terminal sequence support Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase labels Oct 28, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Oct 28, 2019
@zadjii-msft zadjii-msft added Help Wanted We encourage anyone to jump in on these. and removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels Oct 28, 2019
@zadjii-msft zadjii-msft added this to the 21H1 milestone Oct 28, 2019
@ghost ghost added the In-PR This issue has a related PR label Oct 29, 2019
@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Oct 29, 2019
@ghost ghost removed the Help Wanted We encourage anyone to jump in on these. label Oct 29, 2019
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Oct 31, 2019
@ghost
Copy link

ghost commented Nov 26, 2019

🎉This issue was addressed in #3368, which has now been successfully released as Windows Terminal Preview v0.7.3291.0.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-VT Virtual Terminal sequence support Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Conhost For issues in the Console codebase Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants