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

Prevent python class/instance variables from linking to objects #4676

Closed

Conversation

rhelmot
Copy link
Contributor

@rhelmot rhelmot commented Feb 25, 2018

Previously, python variables were given a rolename of obj, which allowed any :ivar foo: or :cvar foo: to link as a cross-reference to any object in the entire build. I believe that this is almost never desirable behavior, since variables often have very simple names that can collide badly with other parts of the project.

Take, for instance, a class with a variable named size. It should not link to a method in a different class that is also named size. A user seeing that the variable is a link would be very confused by what they are linked to.

I believe the solution is to remove the ability for variable names to link anywhere. I'm open to discussion on this one, but I don't the way people write code is such that the name of a variable can be found elsewhere sphinx will know about and will be related in a meaningful way.

…ing :var: from linking to irrelevant objects
@codecov
Copy link

codecov bot commented Feb 25, 2018

Codecov Report

Merging #4676 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4676      +/-   ##
==========================================
- Coverage   81.37%   81.36%   -0.02%     
==========================================
  Files         282      287       +5     
  Lines       37647    37983     +336     
  Branches     5853     5911      +58     
==========================================
+ Hits        30635    30904     +269     
- Misses       5572     5629      +57     
- Partials     1440     1450      +10
Impacted Files Coverage Δ
sphinx/domains/python.py 76.89% <ø> (ø) ⬆️
sphinx/__init__.py 57.5% <0%> (ø)
sphinx/quickstart.py 0% <0%> (ø)
sphinx/apidoc.py 0% <0%> (ø)
sphinx/search/__init__.py 94.34% <0%> (ø)
sphinx/errors.py 67.44% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 64c03f5...b4dc690. Read the comment docs.

@rhelmot
Copy link
Contributor Author

rhelmot commented Mar 25, 2018

Does anyone have any thoughts on this..?

@rhelmot
Copy link
Contributor Author

rhelmot commented Mar 6, 2021

Bump?

@tk0miya
Copy link
Member

tk0miya commented Mar 6, 2021

Sorry for not responding to you long. +1 for applying this.

refs: #8902

@tk0miya tk0miya added this to the 4.0.0 milestone Mar 6, 2021
@tk0miya
Copy link
Member

tk0miya commented Mar 6, 2021

Ah, wait. I fixed this in #8638 without noticing this. Sorry for missing your work and thank you for your contribution.

@tk0miya tk0miya closed this Mar 6, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants