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.

Sign up for GitHub

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

Jump to bottom

Add support for \e escape sequences in c# strings and characters #70497

Merged
merged 14 commits into from Oct 27, 2023
Merged

Add support for \e escape sequences in c# strings and characters #70497

merged 14 commits into from Oct 27, 2023

Conversation

CyrusNajmabadi
Copy link
Member

Language feature approved by the LDM for the any-time bucket: dotnet/csharplang#7400

@CyrusNajmabadi CyrusNajmabadi requested review from a team as code owners October 20, 2023 20:08
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 20, 2023
CyrusNajmabadi
CyrusNajmabadi commented Oct 20, 2023
View reviewed changes
...tiesAndExtensions/Compiler/CSharp/EmbeddedLanguages/VirtualChars/CSharpVirtualCharService.cs
@@ -358,6 +358,7 @@ public override bool TryGetEscapeCharacter(VirtualChar ch, out char escapedChar)
case '0': ch = '\0'; break;
case 'a': ch = '\a'; break;
case 'b': ch = '\b'; break;
case 'e': ch = '\u001b'; break;
Copy link
Member Author

Choose a reason for hiding this comment

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

will add IDE tests as well. But just wanted to get the ball rolling on this.

@jaredpar
Copy link
Member

Don't forget to update language feature status.md

jcouv
jcouv reviewed Oct 20, 2023
View reviewed changes
src/Compilers/CSharp/Test/Syntax/LexicalAndXml/LexicalTests.cs Outdated
"\e"
""";
var value = "\u001b";
var token = LexToken(text, TestOptions.RegularPreview);
Copy link
Member

@jcouv jcouv Oct 20, 2023 •
edited

Choose a reason for hiding this comment

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

RegularPreview is the default. I think you intend to test using RegularNext, which will be updated mechanically to Regular13 once it becomes available. #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

done. thanks!

@build-analysis build-analysis bot mentioned this pull request Oct 22, 2023
Open
3 tasks
jcouv
jcouv reviewed Oct 23, 2023
View reviewed changes
docs/Language Feature Status.md Outdated
@@ -14,6 +14,7 @@ efforts behind them.
| [Params Span\<T> + Stackalloc any array type](https://github.com/dotnet/csharplang/issues/1757) | [params-span](https://github.com/dotnet/roslyn/tree/features/params-span) | [In Progress](https://github.com/dotnet/roslyn/issues/57049) | [cston](https://github.com/cston) | TBD | | [jaredpar](https://github.com/jaredpar) |
| [Default in deconstruction](https://github.com/dotnet/roslyn/pull/25562) | [decon-default](https://github.com/dotnet/roslyn/tree/features/decon-default) | [In Progress](https://github.com/dotnet/roslyn/issues/25559) | [jcouv](https://github.com/jcouv) | [gafter](https://github.com/gafter) | | [jcouv](https://github.com/jcouv) |
| [Roles/Extensions](https://github.com/dotnet/csharplang/issues/5497) | [roles](https://github.com/dotnet/roslyn/tree/features/roles) | [In Progress](https://github.com/dotnet/roslyn/issues/66722) | [jcouv](https://github.com/jcouv) | [AlekseyTs](https://github.com/AlekseyTs), [jjonescz](https://github.com/jjonescz) | | [MadsTorgersen](https://github.com/MadsTorgersen) |
| [Escape character](https://github.com/dotnet/csharplang/issues/7400) | N/A | [In Progress](https://github.com/dotnet/roslyn/pull/70497) | [CyrusNajmabadi](https://github.com/CyrusNajmabadi) | [CyrusNajmabadi](https://github.com/CyrusNajmabadi) | | [CyrusNajmabadi](https://github.com/CyrusNajmabadi) |
Copy link
Member

Choose a reason for hiding this comment

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

CyrusNajmabadi

Put whoever reviews as reviewers (me and someone else)

Copy link
Member Author

Choose a reason for hiding this comment

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

will do. when i get a second reviewer :)

jcouv
jcouv approved these changes Oct 23, 2023
View reviewed changes
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

Compiler side LGTM Thanks (iteration 6)

@jcouv jcouv self-assigned this Oct 23, 2023
@CyrusNajmabadi
Copy link
Member Author

@dotnet/roslyn-compiler @jaredpar for another set of eyes.

@CyrusNajmabadi
Copy link
Member Author

@dotnet/roslyn-compiler @RikkiGibson can i get a set of eyes on this small change? thanks!

@CyrusNajmabadi CyrusNajmabadi merged commit c03aef9 into dotnet:main Oct 27, 2023
26 of 27 checks passed
@ghost ghost added this to the Next milestone Oct 27, 2023
@CyrusNajmabadi CyrusNajmabadi deleted the escapeChar branch October 27, 2023 04:14
@allisonchou allisonchou mentioned this pull request Oct 30, 2023
Closed
@jjonescz jjonescz modified the milestones: Next, 17.9 P1 Oct 31, 2023
@hamarb123 hamarb123 mentioned this pull request Nov 15, 2023
Closed
@Unknown6656 Unknown6656 mentioned this pull request Feb 23, 2024
Open
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RikkiGibson RikkiGibson RikkiGibson approved these changes

@jcouv jcouv jcouv approved these changes

Assignees

@jcouv jcouv

Labels
Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Milestone
17.9 P1
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@CyrusNajmabadi @jaredpar @RikkiGibson @jcouv @jjonescz