[KDevelop/PHP] fix rename of a variable

Authored by hmitonneau on Nov 28 2019, 10:29 AM.



Renaming a variable doesn't work.
This is because in KDevelop::DocumentChangeSetPrivate::generateNewText, rangeText(change.m_range, textLines) return the value $oldname and change.m_oldText has the value oldname

This patch modify the parser to remove the "$" in the range of variable identifiers. As a result, we can now rename a variable, but as a side effect, the "$" is no more highlighted with the variable (the "$" are always blue, and do not take color of the variable)

Test Plan

Right-click on a variable
Select "Rename xxx..."
Choose a new name and click "Rename"

All the occurrences of the variable must be renamed

Diff Detail

R52 KDevelop: PHP Support
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
hmitonneau created this revision.Nov 28 2019, 10:29 AM
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptNov 28 2019, 10:29 AM
hmitonneau requested review of this revision.Nov 28 2019, 10:29 AM
pprkut added a subscriber: pprkut.Dec 14 2019, 9:51 AM

I appreciate the effort, but I think this is the wrong approach. I really think the $ is part of the variable.

I'd say the better approach for this would be to tackle it at the comparison site, to just check for both cases, with and without $. Do you think that's feasible?

This new patch re-implement KDevelop::BasicRefactoring::applyChangesToDeclarations and KDevelop::BasicRefactoring::applyChanges in Php::Refactoring.

The code is a copy/paste of the virtual function, and just modify the range if we detect that it is a var with a "$". This detection is just a comparison between range.length() and name.length()+1

pprkut accepted this revision.Dec 28 2019, 9:19 AM

Looks good :)

This revision is now accepted and ready to land.Dec 28 2019, 9:19 AM
This revision was automatically updated to reflect the committed changes.