search

thSoft / elysium

LilyPond IDE for Eclipse
http://elysium.thsoft.hu
14 stars 3 forks source link

use index for refactoring operations #119

Closed nittka closed 8 years ago

nittka commented 8 years ago

As described in #118 there are severe performance issues in the refactoring operations if the workspace contains many LilyPond source files. This is because in the prior version all the includes of these files where re-evaluated.

I added include information to the index which allows to calculate which source files are affected by a change much faster. At the same time, I changed the refactoring process in a way such that moving multiple files and folders in one refactoring works (fix for #52).

I added preference switches that allow to suppress refactoring warnings that a file contains variable includes (which cannot be refactored as it is unclear which variable is used) as well as the warning that a configured search path is affected by the refactoring (i.e. the search path points to a folder in the workspace and this folder is part of the refactoring which should be a rare case).

I tried to make the refactoring deal with as many cases as possible (relative and absolute includes within the workspace, relative and absolute includes pointing outside the workspace, includes using a search path inside and outside the workspace).

The prior (?) and the current approach have limitation (I'd say severity critical for a case that never occurs). Files outside the workspace are not considered in the refactoring: X is the file containing includes and T is the refactoring target (e.g. a file that is renamed). If there is an include X->T, the reference in X is indexed and updated and everything is fine. If there is an include X->A->T, where A is a file outside the workspace, A is not indexed and hence its reference to T not updated.

nittka commented 8 years ago

The initial refactoring caused the unit tests to fail (as now I am checking whether absolute includes are actually within the ws, but in the unit tests there is no workspace), sorry! Commit 7d8cacfb9f044b21414cf6587053fed074f12e17 fixes this and also adds the regression test for #86, so the corresponding branch is now obsolete.

thSoft commented 8 years ago

Great! Do you plan to push further commits to this PR? If yes, I'd rather cherry-pick 16f4d95 because it addresses a separate issue.

nittka commented 8 years ago

No, no further features, just changes according to your comments... I guess except for the initial commit and 7d8cacfb9f044b21414cf6587053fed074f12e17, they all address different issues ;-)

nittka commented 8 years ago

It's not that I wanted to ignore your last comment about removing the comment from the LilyPond.mwe2 file - I was expecting further comments before you merged the pull requests...