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

[NETBEANS-980] Fix Home/End/Up/Down actions in line-wrapped editor #603

Merged
merged 1 commit into from Sep 15, 2018

Conversation

eirikbakke
Copy link
Contributor

@eirikbakke eirikbakke commented Jun 25, 2018

See https://issues.apache.org/jira/browse/NETBEANS-980 .

This pull request can be reviewed/merged independently of that for NETBEANS-977, which deals with the specific word breaking policy ( #598 ).

@geertjanw geertjanw requested review from sdedic and jlahoda June 26, 2018 07:54
@sdedic
Copy link
Member

sdedic commented Jun 27, 2018

@eirikbakke pls. did you test your feature with the java shell editor ? It does weird things with down/up/begin/end to avoid entering the prompt. Just check if the changes did not break it.

@eirikbakke
Copy link
Contributor Author

I can test that. Will have to get back to this in a little while... in the middle of another deadline and a laptop switch.

@eirikbakke
Copy link
Contributor Author

eirikbakke commented Aug 19, 2018

I have now tested Java Shell both with and without the patch; there seems to be no changes in behavior that I have managed to observed (testing Home/End/Up/Down).

I also tested the case where the current Java Shell prompt line is in fact being wrapped; no new bugs that I could see are being introduced in this case. Here, Home/End/Up/Down seem to work a little differently than in the regular editor, but that was the case before the patch as well.

@eirikbakke
Copy link
Contributor Author

I rebased (and force-pushed) this pull request since changes in the upstream directory structure caused a merge conflict.

I've had the patch enabled (together with #598 ) in both my development IDE and my NetBeans Platform application for about a week now, with no ill effects, so I think both of these pull requests are ready to be merged.

Copy link
Member

@sdedic sdedic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good, but I am not an Editor expert ... go ahead.

@matthiasblaesing
Copy link
Contributor

Merging the approved PR. Thank you both for creating + reviewing this.

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.

None yet

3 participants