Skip to content

Rename support#768

Open
rummatee wants to merge 8 commits into
felixfbecker:masterfrom
rummatee:rename
Open

Rename support#768
rummatee wants to merge 8 commits into
felixfbecker:masterfrom
rummatee:rename

Conversation

@rummatee

Copy link
Copy Markdown

This PR attempts to implement the rename function. Therefore it implements #21 .
There are however still some limitations:
If there isn't a reference in the same file, as the definition, the definition doesn't get changed.
Also at least in my editor (nvim) it is only possible to do one rename and it's necessary to restart before doing the next rename.
I would appreciate any help with these issues or any further comments.

@codecov

codecov Bot commented Nov 15, 2019

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 54.16667% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.14%. Comparing base (9dc1656) to head (02057a4).

Files with missing lines Patch % Lines
src/Server/TextDocument.php 44.64% 31 Missing ⚠️
src/Index/Index.php 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #768      +/-   ##
============================================
- Coverage     82.12%   81.14%   -0.99%     
- Complexity      935      954      +19     
============================================
  Files            44       44              
  Lines          2154     2217      +63     
============================================
+ Hits           1769     1799      +30     
- Misses          385      418      +33     
Files with missing lines Coverage Δ
src/Definition.php 100.00% <ø> (ø)
src/DefinitionResolver.php 87.58% <100.00%> (+0.12%) ⬆️
src/Factory/LocationFactory.php 100.00% <100.00%> (ø)
src/Index/Index.php 76.19% <0.00%> (-1.23%) ⬇️
src/Server/TextDocument.php 64.83% <44.64%> (-10.73%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@felixfbecker felixfbecker left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for taking a stab at this! However, the issues in the description seem like blockers before merging this.

@felixfbecker felixfbecker changed the title Rename Rename support Nov 16, 2019
@rummatee

Copy link
Copy Markdown
Author

I rewrote the definition finding and added code to update the index after the rename. The issues I described earlier should be solved with this.

Comment thread src/Index/Index.php
*/
private function removeIndexedDefinition(int $level, array $parts, array &$storage, array &$rootStorage)
{
if (empty($parts)) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I don't really understand this function well enough to be confident about this code, but it fixes an exception that I got before and it seems intuitively right to have a base case for the recursion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants