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

Remove green flashes after performing regex search #306

Merged
merged 1 commit into from
Oct 14, 2019

Conversation

AlexPl292
Copy link
Contributor

After the performing regex search, small green flashes appear in the editor. Looks like IJ tries to paint the marks with the length of the regex pattern, but it should not happen in regex mode.

With this pull request, I suggest to explicitly set searchWidth to 0 in case of regex search.

Screen Recording 2019-10-08 at 15 01 18

Marker#searchWidth property should be 0 in case of regex search
@AlexPl292
Copy link
Contributor Author

Actually, during my research I've found out that the problem is in the paint() method of the Marker class.

At the very last moment of the jump, IJ decides that it's a nice time to repaint all the markers and invokes paint(). Meanwhile, the AceJump livecycle is already completed and the variables are already reset. That means that Tagger.regex is set to false and the if statement in paint that executed all the time true branch now executes false branch which draws green marks.
So, my initial fix was to store Tagger.regex variable in the constructor and use it as a local property. However, with this approach, a part of the text is greyed out after the jump because during the very last execution of paint the line with composite = getInstance(SRC_OVER, 0.40.toFloat()) is called as well. I was not able to find a fix for the last problem, so I've set searchWidth to 0 in case of regex search, what 1) is technically correct 2) fixes the problem with green lights.

Perhaps the solution where the second branch is not executed at the end would be more correct.

@breandan
Copy link
Collaborator

breandan commented Oct 14, 2019

Hi Alex, this is good work, thanks for investigating. The regex case should have been handled on a separate code path to begin with, but it follows the same path as text search for historical reasons. Since regex search should not accept subsequent search characters, I believe there are a few lingering bugs. Instead of cluttering the search path with control flow for regex, this case should probably take a different path since there is no notion of searchWidth, tag solving, etc.

Indeed, you are right -- there is something fishy going on with composite = getInstance(SRC_OVER, 0.40.toFloat()). I believe if you debug the whole rendering loop, paintMe get called more than is absolutely necessary (the whitish pane during regex search is evidence of it). It is in fact a bug, but we pretend it is a feature. It would be nice to isolate what is the problem exactly.

edit: Re: "At the very last moment of the jump, IJ decides that it's a nice time to repaint all the markers and invokes paint()" Interesting, I was unaware of this.

In general, Marker is a mess and should be completely gutted and refactored. I was looking into replacing Marker with a single CustomHighlightRenderer, which would work, but unfortunately gets painted underneath editor text, so in order to paint tags above text we need a Graphics2D. Perhaps we could investigate if there is another IntelliJ Platform UI component that would do the job and match the platform's look and feel.

@breandan breandan merged commit 01e7ce0 into acejump:master Oct 14, 2019
@AlexPl292 AlexPl292 deleted the GreenLights branch October 15, 2019 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants