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

RD Reparse Before Using Rename #3591

Closed
bbarney213 opened this issue Dec 5, 2017 · 8 comments

Comments

Projects
6 participants
@bbarney213
Copy link

commented Dec 5, 2017

RD should prevent the user from using Rename until the project has been re-parsed. Ideally, RD would re-parse before allowing for a Rename (ideally, check if a reparse is needed first), but at the least there should be a warning, or a disabling of the option. Not reparsing beforehand can result in code changes being removed once the RD rename runs.

@IvenBach

This comment has been minimized.

Copy link
Member

commented Dec 5, 2017

Clean parse with code.
cleanparse

Some edits made without reparsing
lineschangedwithoutparse

Running Rename without reparse produces. bar changed to duk. Note elimination of SomeMethodCalled along with lines being squashed back together.
renamerun

@retailcoder

This comment has been minimized.

Copy link
Member

commented Dec 5, 2017

That's how v1.x worked: before you could use any feature it would reparse, showing a blocking dialog so that there weren't code changes during the parse. As 2.x introduced asynchronous parsing the blocking dialog was eliminated, a manual refresh button was added, and the user is now in charge of deciding when RD should parse.

Now that the parser only processes modules that need to be processed, the cost of a reparse has significantly dropped, but we would have to reimplement the blocking dialog in order for a forced reparse to be of any use.

I'm not sure if it would be an enhancement or a regression, but it's definitely not a bug. Current behavior is entirely by design.

@bbarney213

This comment has been minimized.

Copy link
Author

commented Dec 6, 2017

@retailcoder

This comment has been minimized.

Copy link
Member

commented Dec 6, 2017

Asynchronous/ non-blocking parse cannot guarantee that there aren't any code changes made; parsing before the dialog is shown but without blocking the editor / UI thread wouldn't ensure we're working with the most up-to-date code.

Also the canexecute logic for all commands needs to change to a default return true if every command will trigger a blocking parse.

@bbarney213

This comment has been minimized.

Copy link
Author

commented Dec 6, 2017

@MDoerner

This comment has been minimized.

Copy link
Contributor

commented Dec 6, 2017

I would like to propose a somewhat different approach. We could display a dialog before trying to execute a refactoring command indicating that a reparse is required, provided the loaded projects are dirty at that point. We could also put a reparse now button on the dialog.

One reason why I prefer this to an automatic parse is that, in the eyes of RD, the user might have selected something to refactor that is entirly different from what he intended; the selection that RD sees corresponds to the state of the code at the last reparse.

@bbarney213

This comment has been minimized.

Copy link
Author

commented Dec 6, 2017

@MDoerner

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2019

Fixed by PR #4693

@MDoerner MDoerner closed this Feb 9, 2019

@Vogel612 Vogel612 added this to Done in Release 2.4.1 Feb 28, 2019

@Vogel612 Vogel612 added this to the 2.4.1 milestone Feb 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.