Skip to content

Adding more f-string features #4827

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

Open
samstrohkorbatt wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Adding more f-string features #4827

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f3158be
Fixing issue 4601 and added more features
Feb 10, 2025
899794f
Cleaned up comments
Feb 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 129 additions & 2 deletions src/basic-languages/python/python.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,6 @@ testTokenization('python', [
{ startIndex: 10, type: 'identifier.python' },
{ startIndex: 15, type: 'string.python' },
{ startIndex: 17, type: 'identifier.python' },
{ startIndex: 22, type: 'string.python' },
{ startIndex: 23, type: 'identifier.python' },
{ startIndex: 24, type: 'string.escape.python' }
]
}
Expand All @@ -253,5 +251,134 @@ testTokenization('python', [
{ startIndex: 6, type: 'string.escape.python' }
]
}
],
[
{
line: 'f"{greet(\'Alice\')}"',
tokens: [
{ startIndex: 0, type: 'string.escape.python' },
{ startIndex: 2, type: 'identifier.python' },
{ startIndex: 9, type: 'string.python' },
{ startIndex: 16, type: '' },
{ startIndex: 17, type: 'identifier.python' },
{ startIndex: 18, type: 'string.escape.python' }
]
}
],
[
{
line: "f\"Name: {data['name']}, Age: {data['age']}\"",
tokens: [
{ startIndex: 0, type: 'string.escape.python' },
{ startIndex: 2, type: 'string.python' },
{ startIndex: 8, type: 'identifier.python' },
{ startIndex: 14, type: 'string.python' },
{ startIndex: 20, type: '' },
{ startIndex: 21, type: 'identifier.python' },
{ startIndex: 22, type: 'string.python' },
{ startIndex: 29, type: 'identifier.python' },
{ startIndex: 35, type: 'string.python' },
{ startIndex: 40, type: '' },
{ startIndex: 41, type: 'identifier.python' },
{ startIndex: 42, type: 'string.escape.python' }
]
}
],
[
{
line: 'f"First item: {items[0]}"',
tokens: [
{ startIndex: 0, type: 'string.escape.python' },
{ startIndex: 2, type: 'string.python' },
{ startIndex: 14, type: 'identifier.python' },
{ startIndex: 24, type: 'string.escape.python' }
]
}
],
[
{
line: 'f"{{ Hello }}"',
tokens: [
{ startIndex: 0, type: 'string.escape.python' },
{ startIndex: 2, type: 'string.python' },
{ startIndex: 13, type: 'string.escape.python' }
]
}
],
[
{
line: 'f"{name.capitalize()}"',
tokens: [
{ startIndex: 0, type: 'string.escape.python' },
{ startIndex: 2, type: 'identifier.python' },
{ startIndex: 21, type: 'string.escape.python' }
]
}
],
[
{
line: 'f"{(lambda x: x * 2)(5)}"',
tokens: [
{ startIndex: 0, type: 'string.escape.python' },
{ startIndex: 2, type: 'identifier.python' },
{ startIndex: 24, type: 'string.escape.python' }
]
}
],
[
{
line: 'f"Result: {f\'Value is {value}\'}"',
tokens: [
{ startIndex: 0, type: 'string.escape.python' },
{ startIndex: 2, type: 'string.python' },
{ startIndex: 22, type: 'identifier.python' },
{ startIndex: 29, type: 'string.escape.python' },
{ startIndex: 30, type: 'delimiter.curly.python' },
{ startIndex: 31, type: 'string.escape.python' }
]
}
],

// https://github.com/microsoft/monaco-editor/issues/4601
[
{
line: 'multi_line_f_string = f"""first line looks fine',
tokens: [
{ startIndex: 0, type: 'identifier.python' },
{ startIndex: 19, type: 'white.python' },
{ startIndex: 20, type: '' },
{ startIndex: 21, type: 'white.python' },
{ startIndex: 22, type: 'string.escape.python' },
{ startIndex: 26, type: 'string.python' }
]
},
{
line: "{'uh oh'}",
tokens: [
{ startIndex: 0, type: '' },
{ startIndex: 1, type: 'string.python' },
{ startIndex: 8, type: '' }
]
},
{
line: 'now the highlighting is broken down here :(',
tokens: [{ startIndex: 0, type: 'string.python' }]
},
{
line: '"""',
tokens: [{ startIndex: 0, type: 'string.escape.python' }]
},
{
line: 'also = "it\'s broken highlighting for everything after"',
tokens: [
{ startIndex: 0, type: 'identifier.python' },
{ startIndex: 4, type: 'white.python' },
{ startIndex: 5, type: '' },
{ startIndex: 6, type: 'white.python' },
{ startIndex: 7, type: 'string.escape.python' },
{ startIndex: 8, type: 'string.python' },
{ startIndex: 53, type: 'string.escape.python' }
]
}
]
]);
50 changes: 42 additions & 8 deletions src/basic-languages/python/python.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,16 +250,32 @@ export const language = <languages.IMonarchLanguage>{
// Recognize strings, including those broken across lines with \ (but not without)
strings: [
[/'$/, 'string.escape', '@popall'],
[/f'{1,3}/, 'string.escape', '@fStringBody'],
[/f'{3}/, 'string.escape', '@fStringBody3'],
[/f'{1}/, 'string.escape', '@fStringBody1'],
[/'/, 'string.escape', '@stringBody'],
[/"$/, 'string.escape', '@popall'],
[/f"{1,3}/, 'string.escape', '@fDblStringBody'],
[/f"{3}/, 'string.escape', '@fDblStringBody3'],
[/f"{1}/, 'string.escape', '@fDblStringBody1'],
[/"/, 'string.escape', '@dblStringBody']
],
fStringBody: [
fStringBody3: [
[/[^\\'\{\}]+/, 'string'],
Copy link
Contributor

@aiday-mar aiday-mar May 7, 2025

Choose a reason for hiding this comment

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

I have several questions about the PR which I would like to ask to better understand this PR.

How come here on line 262 it's written

[/[^\\'\{\}]+/, 'string']

But on line 273 we have:

		[/[^\\'\{\}]+$/, 'string', '@popall'],
		[/[^\\'\{\}]+/, 'string'],

For the other case? It seems like this code is not the exact duplicate of the other code, as it does not contain the third value '@popall'.

Also would you know what popall here refers to? I am trying to understand.

[/\{\{[^}]+\}\}/, 'string'],
[/\{f"{3}/, 'string', '@fDblStringBody3'], // for nested f-strings
[/\{f"{1}/, 'string', '@fDblStringBody1'], // for nested f-strings
[/\{[^\}'":!=]+/, 'identifier', '@fStringDetail'],
[/\\./, 'string'],
[/'''/, 'string.escape', '@popall'],
Copy link
Contributor

@aiday-mar aiday-mar May 7, 2025

Choose a reason for hiding this comment

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

Why was this added? The equivalent of this does not seem to be present in the fStringBody1 array. What use case does it fix?

[/'/, 'string.escape'],
[/\\$/, 'string']
],
fStringBody1: [
[/[^\\'\{\}]+$/, 'string', '@popall'],
[/[^\\'\{\}]+/, 'string'],
[/\{[^\}':!=]+/, 'identifier', '@fStringDetail'],
[/\{\{[^}]+\}\}/, 'string'],
[/\{f"{3}/, 'string', '@fDblStringBody3'], // for nested f-strings
[/\{f"{1}/, 'string', '@fDblStringBody1'], // for nested f-strings
[/\{[^\}'":!=]+/, 'identifier', '@fStringDetail'],
[/\\./, 'string'],
[/'/, 'string.escape', '@popall'],
[/\\$/, 'string']
Expand All @@ -271,10 +287,24 @@ export const language = <languages.IMonarchLanguage>{
[/'/, 'string.escape', '@popall'],
[/\\$/, 'string']
],
fDblStringBody: [
fDblStringBody3: [
[/[^\\"\{\}]+/, 'string'],
[/\{\{[^}]+\}\}/, 'string'],
[/\{f'{3}/, 'string', '@fStringBody3'], // for nested f-strings
[/\{f'{1}/, 'string', '@fStringBody1'], // for nested f-strings
[/\{[^\}'":!=]+/, 'identifier', '@fStringDetail'],
[/\\./, 'string'],
[/"""/, 'string.escape', '@popall'],
[/"/, 'string.escape'],
Copy link
Contributor

@aiday-mar aiday-mar May 7, 2025

Choose a reason for hiding this comment

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

So we escape on the " and on the """. What use case does it fix? And why does line 298 not have @popall as third value in the array?

[/\\$/, 'string']
],
fDblStringBody1: [
[/[^\\"\{\}]+$/, 'string', '@popall'],
[/[^\\"\{\}]+/, 'string'],
[/\{[^\}':!=]+/, 'identifier', '@fStringDetail'],
[/\{\{[^}]+\}\}/, 'string'],
[/\{f'{3}/, 'string', '@fStringBody3'], // for nested f-strings
[/\{f'{1}/, 'string', '@fStringBody1'], // for nested f-strings
[/\{[^\}'":!=]+/, 'identifier', '@fStringDetail'],
[/\\./, 'string'],
[/"/, 'string.escape', '@popall'],
[/\\$/, 'string']
Expand All @@ -287,9 +317,13 @@ export const language = <languages.IMonarchLanguage>{
[/\\$/, 'string']
],
fStringDetail: [
[/[:][^}]+/, 'string'],
[/:=[^}]+/, 'identifier'], // walrus operator
Copy link
Contributor

@aiday-mar aiday-mar May 7, 2025

Choose a reason for hiding this comment

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

Can you give me examples of where these would activate? Why are they followed by 'identifier' and not string?

[/: [^}]+/, 'identifier'], // for lambda functions
[/:[^}]+/, 'string'], // for numeric formatting
[/[!][ars]/, 'string'], // only !a, !r, !s are supported by f-strings: https://docs.python.org/3/tutorial/inputoutput.html#formatted-string-literals
[/=/, 'string'],
[/=/, 'identifier'],
Copy link
Contributor

@aiday-mar aiday-mar May 7, 2025

Choose a reason for hiding this comment

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

Why was this changed from string to identifier?

[/'[^\']+'/, 'string'],
[/"[^\""]+"/, 'string'],
Copy link
Contributor

@aiday-mar aiday-mar May 7, 2025

Choose a reason for hiding this comment

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

Should this be instead

/"[^\"]+"/

?

[/\}/, 'identifier', '@pop']
]
}
Expand Down
Loading