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

Used f-strings instead of percent formatting. #1934

Closed
wants to merge 5 commits into from

Conversation

tahadnan
Copy link

@tahadnan tahadnan commented Nov 2, 2024

This pull request modernizes the string formatting in the documentation and example files of prompt_toolkit by replacing % formatting with f-strings. This update enhances readability and aligns the code with Python's current best practices, as f-strings provide a more concise and efficient way to format strings.

Changes Made

  • Replaced % formatting with f-strings in various documentation examples and code snippets.
  • Improved readability by reducing concatenation and making formatting more consistent.

Motivation

Switching to f-strings is beneficial because:

  • f-strings are more readable and intuitive, especially for beginners.
  • They improve code maintainability by reducing the need for tuple wrapping in cases of single variables.
  • f-strings offer slight performance improvements over % formatting, which is valuable for code examples in a widely-used library like prompt_toolkit.

Testing

  • Ran modified examples to confirm they work as expected with the updated f-string formatting.
  • No functional changes to the examples' behavior, only a formatting improvement.

Additional Notes

No new dependencies or additional code changes are introduced. This PR solely focuses on modernizing string formatting in examples.

@jonathanslenders
Copy link
Member

I prefer to not merge this. In many cases it's fine, but I see you also changed the formatting approach of the HTML() class, which will result in injection vulnerabilities or broken code due to incorrect escaping. We need PEP 750 for this.

I'd also prefer if the replacing of %-style formatting with f-strings were not done in the same PR as typo corrections, and if the replacement was performed automatically with a tool like pyupgrade. That way, the replacement is only done in places where it is fine.

@tahadnan
Copy link
Author

tahadnan commented Nov 4, 2024

So should I just close this PR?

@jonathanslenders
Copy link
Member

jonathanslenders commented Nov 4, 2024

Yes please, but thanks for the effort anyway!

Regarding the commit with the typos, one typo is a typo, the other isn't. It is "Vertica" not "vertical". Feel free to create a separate PR for this.
About the formatting, I'll try to remember doing pyupgrade over the codebase on the next change.

@tahadnan
Copy link
Author

tahadnan commented Nov 5, 2024

Great! thanks for the smooth communication and I really appreciate this project guys, keep up the good work.

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.

2 participants