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 basic ANSI escape character control support #204

Closed
wants to merge 5 commits into from
Closed

Add basic ANSI escape character control support #204

wants to merge 5 commits into from

Conversation

nununoisy
Copy link

Add support for readability in serial terminal

Commands:
^[J - normally clears screen to the right, simply ignored here
^[H^[J - home cursor, clear screen to the right - clears textarea

Add support for readability in serial terminal

Commands:
^[J - normally clears screen to the right, simply ignored here
^[H^[J - home cursor, clear screen to the right - clears textarea
Copy link
Owner

@copy copy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, highly appreciated. We need a few changes before I can merge this.

else if(chr === "\r")
{
// do nothing
if(this.control_mode === false) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please follow the general style of the code ({ on an empty line after the )).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried so hard to match the code style, it's nothing like my own. Will fix.

}
else if(cmds.invalid.test(this.control_buffer))
{
this.control_mode = false;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably we should just add the escape sequence to the text here to make it easier to see when they're not handled.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see that. Alright.

var cmds = {
bksp: /$\[.J^/,
clr: /$\[.H\x1B\[.J^/,
invalid: /$*[^HJ]^/
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These don't look quite right:

  • $ matches the end of the text and ^ the beginning
  • The * in invalid probably requires something before it
  • I believe invalid would match before the other two; it needs to be more specific
  • Instead of the ., we should probably use a more specific match. This page says it can be 1, 2 or 3, so [123] might be more appropriate

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops! I'm still getting the hang of regex. Silly mistakes.

@nununoisy
Copy link
Author

Alright. I've made the requested changes. Let me know what you think.

this.text = this.text.slice(0, -1);
this.update();
}
else if(chr === "\x1b") {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing newline before {.

var cmds = {
bksp: /^\[[0-2]?J$/,
clr: /^\[[0-9;]{0,5}H\x1b\[[0-2]?J$/,
invalid: /[hl=>0-2M-OmrA-Efg3-8KnRc8qy]/
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really hard to understand, can you find another way to find the end of the control buffer? If not, I can give this a shot.

Maybe a single regular expression could be used to match any sequence as described here:

The ESC [ is followed by any number (including none) of "parameter bytes" in the range 0x30–0x3F (ASCII 0–9:;<=>?), then by any number of "intermediate bytes" in the range 0x20–0x2F (ASCII space and !"#$%&'()*+,-./), then finally by a single "final byte" in the range 0x40–0x7E (ASCII @A–Z[]^_`a–z{|}~).

Copy link
Author

@nununoisy nununoisy Apr 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could go off of that spec. I created that regex for compatibility with VT-100 commands as well. If need be I will fix it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might just wait for this PR to go through and write a new version of this file with screen memory so I can implement more commands

@nununoisy
Copy link
Author

nununoisy commented Apr 9, 2018

Alright @copy. Maybe this time. I’m working on a new branch with screen memory so I can implement complex commands.

@copy
Copy link
Owner

copy commented Jul 13, 2018

Closing as no progress in 3 months.

@copy copy closed this Jul 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants