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

Use of old-style format leads to advice with broken link #115

Closed
richiemorrisroe opened this issue Sep 4, 2020 · 4 comments · Fixed by #120
Closed

Use of old-style format leads to advice with broken link #115

richiemorrisroe opened this issue Sep 4, 2020 · 4 comments · Fixed by #120

Comments

@richiemorrisroe
Copy link

richiemorrisroe commented Sep 4, 2020

Hey,

Thanks for the tool.

There appears to be an invalid link in (at least) one of the advices.

Note the attached file.

When running python -m fixit.cli.run_rules with a file with the following contents:

myname = "richie"
string_old = "This is {name}'s string".format(name=myname)

string_new = f"This is {myname}'s string"

I get the following advice:

invalid_link_suggestion.py:2:14
UseFstringRule: Do not use printf style formatting or .format(). Use f-string instead to be more readable and
efficient. See https://fburl.com/usefstring.```

I mean, I like fburls, I remember them well. However, I (and pretty much every non-FB employee) have no access to them.

It would probably be better if this pointed to a link accessible from the public web.

@jimmylai
Copy link
Contributor

jimmylai commented Sep 4, 2020

Nice catch! We should not use fburl here.

Ideally, we can have the link automatically generated by Fixit core as f"https://fixit.readthedocs.io/en/latest/{rule_name}.html" and append to the message at runtime.

@acharles7
Copy link
Contributor

acharles7 commented Sep 5, 2020

I think https://www.python.org/dev/peps/pep-0498/ will be a replacement for fburl
I can contribute to this if the above link is an alternative to fburl for string rule.

@jimmylai
Copy link
Contributor

jimmylai commented Sep 6, 2020

@acharles7 , the PEP provides f-string usage examples and looks good as a replacement to the existing link. Feel free to submit a PR!

@acharles7
Copy link
Contributor

Thanks, will make PR soon.

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 a pull request may close this issue.

3 participants