-
Notifications
You must be signed in to change notification settings - Fork 160
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
Fix ViewString
for string to correctly escape characters needing it
#3693
Conversation
658ace8
to
8b2f299
Compare
src/stringobj.c
Outdated
** | ||
** 'PrintString' prints the string with the handle <list>. | ||
** 'CHUNK_ESCAPE_STRING' returns a list of strings which, when concatenated, | ||
** produce a String containing what PrintString outputs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I don't understand: Why not instead provide a function VIEW_STRING_FOR_STRING
which returns the already concatenated result? Do you have any applications in mind where one would need these separate "chunks"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, just generally trying to make the C-level code as simple as possible
Seems this needs to be rebased now, too. Also, labels need to be set (at the very least a "release notes" label) |
8b2f299
to
f96cb02
Compare
@ChrisJefferson I updated my commit to fix a comment; otherwise, this is ready to merge from my POV, but since I modified your PR, I do not want to merge this w/o your explicit approval (in particular, if you do not approve, then I don't want to merge it). |
Sorry, I was ignoring it while the tests still failed, but I notice this is just jenkins/gap-system being badly behaved. I'm happy with this. |
ViewString
to correctly escape strings
ViewString
to correctly escape stringsViewString
for string to correctly escape characters needing it
Fixes #2884
This fixes ViewString not correctly escaping strings. This is done by making the function which prints strings to the terminal generic, so we can print a string (with escape characters) to a buffer.
Note that, to make the kernel level stuff easier, I print to a plist of strings which have to be concatenated together, which then actually gets done back at the GAP level. If this offends anyone I could make a growing string and do this in one go, but this seemed easier.