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

Improved responsiveness of editor for large projects #12

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rimesc
Copy link
Contributor

@rimesc rimesc commented Apr 25, 2014

The attached change goes a long way towards improving the responsiveness of the editor for projects with large numbers of steps (issue #6), by not recreating the same StepMatcher for every candidate step on every keystroke.

each) and recreating and recompiling the regex pattern every time is
insanely expensive.
@benheap
Copy link

benheap commented Apr 28, 2014

Please can this pull request be assessed and accepted as soon as possible as we have been waiting a long time for a change that would improve this plugin's responsiveness

@shayzluf
Copy link

We would like you to merge the fix as soon as possible

@nevoalm
Copy link

nevoalm commented Apr 28, 2014

it would be great to get the changes!!!

@snipem
Copy link

snipem commented May 6, 2014

We are waiting for this aswell

Update:
I was able to build the Eclipse plugin for myself based on rimesc's fork and the instructions in BUILD.markdown.
The JBehave Eclipse editor is now fast even with big stories (270 scenarios in one file). Please accept the patch, so that JBehave is usable in a production environment eventually.

Remarks:
I've used a Linux based Maven 3.0.5 build. My first attempt with Maven 3.2.1 failed due to an exception raising from Tycho (see https://stackoverflow.com/questions/18058419/maven-nattable-cleaninstall-failure). This behaviour is likely to be unrelated with rimesc's patch.

Update:
Searching with CTRL+F isn't working properly

@benheap
Copy link

benheap commented May 19, 2014

Still waiting for this pull request to be merged, are there concerns around it? I believe the CTRL+F issue mentioned in the comment above was pre-existing

@dertseha
Copy link
Contributor

@benheap as far as I remember, PRs are only considered if there is a corresponding issue in their tracker open. Did you create one?
(Yes, I'd also appreciate a performance boost :)

@benheap
Copy link

benheap commented May 30, 2014

@dertseha I raised #1 a good while back (as you can tell from the issue number :) ) but don't know if thats in the pull request

@benheap
Copy link

benheap commented May 30, 2014

to be honest, most of our team are using IntelliJ now anyway but for those still using eclipse this does affect them in their day to day writing of stories

@rimesc
Copy link
Contributor Author

rimesc commented Jun 1, 2014

I referenced issue #6 in the pull request because the profiling data (screenshot) attached to that bug matches what I was seeing and what the changes should address. It may also address issue #1 although it obviously only fixes one of the possible performance bottlenecks.

@dertseha
Copy link
Contributor

dertseha commented Jun 4, 2014

@benheap I meant the Jira tracker of the project, which is used as a base for everything. But Paul Burton over there just re-referenced this issue here: https://jira.codehaus.org/browse/JBEHAVE-889

@paul-barton
Copy link

I created a new improvement request in codehaus for this pull request: https://jira.codehaus.org/browse/JBEHAVE-1023

might be a reference to a constant or an expression) by setting the
priority to null.
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.

7 participants