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

Terminal support for handling color text display with ANSI escape sequences #154

Open
SimonGAndrews opened this issue Oct 20, 2022 · 4 comments

Comments

@SimonGAndrews
Copy link

SimonGAndrews commented Oct 20, 2022

Formalising suggested enhancement to provide support for the display of coloured text in the terminal window of the WebIDE (and other usage of the core terminal module) as per forum discussion https://forum.espruino.com/conversations/380403/#comment16713624.
Suggestion is to allow console.log from Espruino to embed standard ANSI/VT100 set/reset display attribute sequences to enable changes to foreground and background colours of the text displayed in the terminal window.

@SimonGAndrews
Copy link
Author

SimonGAndrews commented Oct 20, 2022

Happy to be assigned the task

The ansi/VT100 standards allow for the the escape sequences 'esc [ nnn m; to be used to set/reset the foreground and background colour of text.
image

image

image

Investigations show the change can be effected in https://github.com/espruino/EspruinoTools/blob/master/core/terminal.js. In summary the new escape sequences need to be trapped and used to effect the creation of additional HTML to be sent to the terminal canvas in the browser plugin.
The existing 3rd party library https://github.com/osteele/terminal-codes-to-html under MIT license provides the code necessary to convert the ANSI control sequences into HTML spans with style=colour attributes.

In more detail terminal.js functions handleRecievedCharacter() and updateTerminal() will be modified and a new function terminalCodesToHtml() (a modified version of Osteele's library) will be added.

  • Function handleRecievedCharacter() will be restructured to allow longer and more varied escape sequences to be identified. The function will allow valid escape sequences which are to be converted to HTML to be written to termText[] for subsequent processing by terminalCodesToHtml().
  • The function addCharacters() is created from code in handleRecievedCharacter() for a reusable method to update termText[]
  • The function updateTerminal() will be changed to add a call to terminalCodesToHtml() passing the terminal line to be tested for the creation of colour styled spans from the colour control escape sequences. Not unlike how link URLs are currently converted.

Initial versions of these changes have been made and will be tested in the fork https://github.com/SimonGAndrews/EspruinoTools

Notes on testing to follow here (all comments welcome)

@SimonGAndrews SimonGAndrews changed the title Terminal support for handling color test display with ANSI escape sequences Terminal support for handling color text display with ANSI escape sequences Oct 20, 2022
@gfwilliams
Copy link
Member

gfwilliams commented Oct 21, 2022

This sounds like a great idea to me.

However, I'm not entirely sure about: master...SimonGAndrews:EspruinoTools:master

Specifically I'm very unsure about including escape characters in termText. I think this will break other things (clicking, deleting, and also stepping over coloured text with arrow keys) as we assume that termCursorX points to the column of chars, which is in termText[termCursorY][termCursorX].

As far as I can tell if you set the cursor color, right now it only sets the color for that line, not for subsequent lines?

Also...

  • It seems if (termControlChars[0]=='27' && termControlChars[1]=='91') { // Esc [ is quoted when it shouldn't be?
  • I'm not a great fan of the repeated /\x1B\[0m/.test(String.fromCharCode.apply(null, termControlChars)) - this seems pretty verbose and inefficient when just if (termControlChars[0]==.. && termControlChars[1]==.. ... ) would work (and is used in other places) - or even just a string compare. I know for things like /\x1B\[[3-4][0-7]m/ it makes a bit more sense, but I guess termControlChars could be made a string rather than an array and then the checks could on the whole just be string compares with the occasional regex?
  • Generally I'd prefer not to copy in code with different licences, especially when it's only a few lines worth. It's ok, but because of what I mentioned above with termText I wonder if we'd be better off with a different approach anyway?

I'm open to suggestions, but it feels like in handleReceivedCharacter we basically decode the escape codes anyway, so it might be better to figure out what they are meant to do in handleReceivedCharacter, and then store them separately.

For each terminal line we could have an array of {position,color} objects, and then when we're generating the HTML I guess we could fix up the output line as we go along?

@SimonGAndrews
Copy link
Author

SimonGAndrews commented Oct 21, 2022

ok, thanks Gordon,
will take another look with your feedback.
I do like the library , its a pretty elegant way to get the HTML created.
But yes it does only colour the current line , its not modal across all lines as a VT100 would behave. This was good enough for simple console.log. Is ther another use case to test ?

Ill get a test for the areas you suggest would break, as Ive not seen that in a node standalone build of the WebIDE.
(by the way the term text insert of escape sequences is as characters with cursor variables adjusted, but im probably missing something ).

What other builds would need testing ?

will have another go next week.

@SimonGAndrews
Copy link
Author

SimonGAndrews commented Oct 22, 2022

Ok slept on this and will abandon current approach.
Will try your idea:
‘For each terminal line we could have an array of {position,color} objects, and then when we're generating the HTML I guess we could fix up the output line as we go along?’

SimonGAndrews added a commit to SimonGAndrews/EspruinoTools that referenced this issue Nov 10, 2022
SimonGAndrews added a commit to SimonGAndrews/EspruinoTools that referenced this issue Nov 10, 2022
SimonGAndrews added a commit to SimonGAndrews/EspruinoTools that referenced this issue Nov 11, 2022
SimonGAndrews added a commit to SimonGAndrews/EspruinoTools that referenced this issue Nov 11, 2022
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

No branches or pull requests

2 participants