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 SyntaxWarning: invalid escape sequence '\{' #2777

Merged

Conversation

dwrobel
Copy link
Contributor

@dwrobel dwrobel commented Dec 11, 2023

Fixes the following syntax warning on python 3.12:

/usr/share/qtvcp/screens/qtplasmac_4x3/qtplasmac_4x3_handler.py:4016:
SyntaxWarning: invalid escape sequence '{'
commands = f"{data[1]}{data[3]}"
/usr/share/qtvcp/screens/qtplasmac_4x3/qtplasmac_4x3_handler.py:4804:
SyntaxWarning: invalid escape sequence '{'
msgs = f'{msg0}.\n\n{msg1}:'

@@ -4801,7 +4801,7 @@ def delete_material_clicked(self):
except:
if not matNum:
msg0 = _translate('HandlerClass', 'A material number is required')
msgs = f'{msg0}.\n\n\{msg1}:'
msgs = f'{msg0}.\n\n\\{msg1}:'
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
msgs = f'{msg0}.\n\n\\{msg1}:'
msgs = f'{msg0}.\n\n{msg1}:'

Could this also have been the author's intention? I have no idea how these commands should look like.

Copy link
Contributor Author

@dwrobel dwrobel Dec 11, 2023

Choose a reason for hiding this comment

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

Could this also have been the author's intention?

I would prefer not to guess rather wait for the author to comment.
For sure the current version is wrong.

Copy link
Contributor

@snowgoer540 snowgoer540 Dec 11, 2023

Choose a reason for hiding this comment

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

Could this also have been the author's intention? I have no idea how these commands should look like.

Yes, this was a fairly long standing typo, thanks for noticing; hansu's version is correct:

msgs = f'{msg0}.\n\n{msg1}:'

@snowgoer540
Copy link
Contributor

I agree with the change to line 4016, and hansu's changes as noted.

Please make sure to update the version at line 1, and then update the version.html file in the same folder.

@dwrobel dwrobel force-pushed the Fix-SyntaxWarning-invalid-escape-sequence branch from 262bad2 to 0f48d6c Compare December 12, 2023 12:42
@snowgoer540
Copy link
Contributor

Thanks for making the changes.

Not to be pedantic, but can you fix the date on this line, and then I'm happy to merge it:

<br><b><u>001.012 2023 Nov 16</u></b>

@dwrobel dwrobel force-pushed the Fix-SyntaxWarning-invalid-escape-sequence branch from 0f48d6c to ff400cc Compare December 12, 2023 16:38
@dwrobel
Copy link
Contributor Author

dwrobel commented Dec 12, 2023

but can you fix the date on this line

Fixed.

@snowgoer540
Copy link
Contributor

I'm sorry, I didnt notice you changed line 4016 as well. It was correct in your first commit.

It should be:

commands = f"{data[1]}\\{data[3]}"

Only the line Hansu pointed out needed changed.

 Fixes the following syntax warning on python 3.12:

 /usr/share/qtvcp/screens/qtplasmac_4x3/qtplasmac_4x3_handler.py:4016:
  SyntaxWarning: invalid escape sequence '\{'
   commands = f"{data[1]}\{data[3]}"
 /usr/share/qtvcp/screens/qtplasmac_4x3/qtplasmac_4x3_handler.py:4804:
  SyntaxWarning: invalid escape sequence '\{'
   msgs = f'{msg0}.\n\n\{msg1}:'

Signed-off-by: Damian Wrobel <[email protected]>
@dwrobel dwrobel force-pushed the Fix-SyntaxWarning-invalid-escape-sequence branch from ff400cc to 44b91df Compare December 12, 2023 17:40
@dwrobel
Copy link
Contributor Author

dwrobel commented Dec 12, 2023

I'm sorry, I didnt notice you changed line 4016 as well. It was correct in your first commit.

Original version restored.

@snowgoer540
Copy link
Contributor

Thank you! As soon as the checks finish, I will merge this one.

@snowgoer540 snowgoer540 merged commit 7d9b5e5 into LinuxCNC:master Dec 12, 2023
11 checks passed
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

Successfully merging this pull request may close these issues.

3 participants