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

Fix ViewString for string to correctly escape characters needing it #3693

Merged
merged 2 commits into from
Nov 6, 2019

Conversation

ChrisJefferson
Copy link
Contributor

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.

@coveralls
Copy link

coveralls commented Oct 7, 2019

Coverage Status

Coverage increased (+0.03%) to 84.508% when pulling 95e88be on ChrisJefferson:viewstring into 2c596b3 on gap-system:master.

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.
Copy link
Member

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"?

Copy link
Contributor Author

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

src/stringobj.c Outdated Show resolved Hide resolved
src/stringobj.c Outdated Show resolved Hide resolved
@fingolfin
Copy link
Member

Seems this needs to be rebased now, too. Also, labels need to be set (at the very least a "release notes" label)

@ChrisJefferson ChrisJefferson added kind: bug: unexpected error Issues describing bugs in which computation unexpectedly encounters an error, and PRs fixing them release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Oct 14, 2019
@ChrisJefferson ChrisJefferson changed the title Fix escaping Strings in ViewString Improve ViewString so Strings are correctly escaped Oct 14, 2019
@fingolfin
Copy link
Member

@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).

@ChrisJefferson
Copy link
Contributor Author

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.

@fingolfin fingolfin merged commit a3035e7 into gap-system:master Nov 6, 2019
@ChrisJefferson ChrisJefferson deleted the viewstring branch December 2, 2019 14:30
@ThomasBreuer ThomasBreuer self-assigned this Feb 17, 2021
@ThomasBreuer ThomasBreuer added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Feb 17, 2021
@ThomasBreuer ThomasBreuer removed their assignment Feb 17, 2021
@fingolfin fingolfin changed the title Improve ViewString so Strings are correctly escaped Fix ViewString to correctly escape strings Aug 17, 2022
@fingolfin fingolfin changed the title Fix ViewString to correctly escape strings Fix ViewString for string to correctly escape characters needing it Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug: unexpected error Issues describing bugs in which computation unexpectedly encounters an error, and PRs fixing them release notes: added PRs introducing changes that have since been mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ViewString does not correctly escape quotes inside strings
5 participants