Skip to content

[READY] Revert #1275 #1343

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

Merged
merged 1 commit into from
Oct 19, 2019
Merged

[READY] Revert #1275 #1343

merged 1 commit into from
Oct 19, 2019

Conversation

bstaletic
Copy link
Collaborator

@bstaletic bstaletic commented Oct 19, 2019

PR #1275 broke GoTo in python in more ways than one. See:

Changes in the behaviour:

  • GoTo, GoToDefinition and GoToDeclaration now do what GoToType used to
    do.
  • We don't expose jedi's goto_assignments which can be useful, but we
    do have GoToReferences that calls jedi's usages().
    • This means that GoTo on self.x will now jump to the definition
      of the type of x, while GoToReferences will do what is expected.
    • The goto_usages for this case returns "weird" locations, see PR #1275 broke GoTo for member functions #1341
      for details.

There's a test for member function access that was broken before.


This change is Reviewable

PR ycm-core#1275 broke GoTo in python in more ways than one. See:

- ycm-core/YouCompleteMe#3458
- ycm-core#1341

Changes in the behaviour:

- GoTo, GoToDefinition and GoToDeclaration now do what GoToType used to
do.
- We don't expose jedi's `goto_assignments` which can be useful, but we
do have GoToReferences that calls jedi's `usages()`.
  - This means that `GoTo` on `self.x` will now jump to the definition
of the type of `x`, while `GoToReferences` will do what is expected.
  - The `goto_usages` for this case returns "weird" locations, see ycm-core#1341
for details.
@codecov
Copy link

codecov bot commented Oct 19, 2019

Codecov Report

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

@@            Coverage Diff             @@
##           master    #1343      +/-   ##
==========================================
- Coverage   97.17%   97.16%   -0.02%     
==========================================
  Files          96       96              
  Lines        7481     7446      -35     
  Branches      175      153      -22     
==========================================
- Hits         7270     7235      -35     
  Misses        170      170              
  Partials       41       41

Copy link
Member

@puremourning puremourning left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 4 of 4 files at r1.
Reviewable status: 1 of 2 LGTMs obtained

@bstaletic bstaletic added the Ship It! Manual override to merge a PR by maintainer label Oct 19, 2019
@mergify mergify bot merged commit 19359f6 into ycm-core:master Oct 19, 2019
@mergify
Copy link
Contributor

mergify bot commented Oct 19, 2019

Thanks for sending a PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ship It! Manual override to merge a PR by maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants