You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "mbien (via GitHub)" <gi...@apache.org> on 2024/07/06 23:35:30 UTC
[PR] Introduce-method refactoring should suggest a method name [netbeans]
mbien opened a new pull request, #7551:
URL: https://github.com/apache/netbeans/pull/7551
if the test field is left empty, it creates the following problem:
- cancel button has focus, ok button is disabled until validaiton
- method name validation is delayed by 200ms which can cause users to cancel the hint after filling out the method name and pressing enter quickly, giving the wrong impression of a not functioning refactoring action
This can be avoided by initializing the test field with a method name.
how to test:
- select `System.out.println();`
- `alt+enter`, select `introduce method`
- type something and hit enter quickly
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Introduce-method refactoring should suggest a method name [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #7551:
URL: https://github.com/apache/netbeans/pull/7551#issuecomment-2234179513
@neilcsmith-net I looked through the hierarchy and added `btnCancel.setDefaultCapable(false);` to all fixes I found.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Introduce-method refactoring should suggest a method name [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #7551:
URL: https://github.com/apache/netbeans/pull/7551#issuecomment-2248675624
planning to merge this Thursday morning UTC a day before freeze.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Introduce-method refactoring should suggest a method name [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #7551:
URL: https://github.com/apache/netbeans/pull/7551#discussion_r1684655529
##########
java/java.hints/src/org/netbeans/modules/java/hints/introduce/IntroduceMethodPanel.java:
##########
@@ -254,6 +253,7 @@ private Preferences getPreferences() {
public void setOkButton( JButton btn ) {
this.btnOk = btn;
+ changeSupport.run();
Review Comment:
it is quite fast even in large files but I agree in principle. It is also run on `focusLost()` etc, so this would require more work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Introduce-method refactoring should suggest a method name [netbeans]
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on code in PR #7551:
URL: https://github.com/apache/netbeans/pull/7551#discussion_r1683894034
##########
java/java.hints/src/org/netbeans/modules/java/hints/introduce/IntroduceMethodPanel.java:
##########
@@ -254,6 +253,7 @@ private Preferences getPreferences() {
public void setOkButton( JButton btn ) {
this.btnOk = btn;
+ changeSupport.run();
Review Comment:
On other places, ChangeSupport.run is run off EDT, since validators tend to use Parsing API which might be quite slow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Introduce-method refactoring should suggest a method name [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien merged PR #7551:
URL: https://github.com/apache/netbeans/pull/7551
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Introduce-method refactoring should suggest a method name [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #7551:
URL: https://github.com/apache/netbeans/pull/7551#issuecomment-2217152850
> Could also look at setting btnCancel.setDefaultCapable(false); ??
this would be better than cancelling the dialog, however it could create the situation that pressing enter would do nothing the first time its pressed. If the user tries again 200ms later it would work - still not ideal
The trick here is to have the initial state of the dialog to be valid, like the other "introduce" dialogs. Those are able to suggest sensible input values, with the introduce-method refactoring this is more difficult.
This does also validate the default right away without delay, so it will disable the OK button if there is a conflict.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Introduce-method refactoring should suggest a method name [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #7551:
URL: https://github.com/apache/netbeans/pull/7551#discussion_r1684809866
##########
java/java.hints/src/org/netbeans/modules/java/hints/introduce/IntroduceMethodPanel.java:
##########
@@ -254,6 +253,7 @@ private Preferences getPreferences() {
public void setOkButton( JButton btn ) {
this.btnOk = btn;
+ changeSupport.run();
Review Comment:
so i debugged this and the whole panel initialization runs off-EDT(!), this call included ;)
What I did though to not make it worse: I removed this call to `run()` again, and triggered (async)validation when the default value is set.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Introduce-method refactoring should suggest a method name [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #7551:
URL: https://github.com/apache/netbeans/pull/7551#issuecomment-2231339737
rebase just to refresh since a lot happened in the meantime
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Introduce-method refactoring should suggest a method name [netbeans]
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on PR #7551:
URL: https://github.com/apache/netbeans/pull/7551#issuecomment-2217071031
Seems an OK quick fix. Those should ideally be refactored to use validity support in dialogs? Problem seems caused by bypassing that and trying to achieve it manually? https://bits.netbeans.org/22/javadoc/org-openide-dialogs/org/openide/NotifyDescriptor.html#setValid-boolean-
Could also look at setting `btnCancel.setDefaultCapable(false);` ??
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Introduce-method refactoring should suggest a method name [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #7551:
URL: https://github.com/apache/netbeans/pull/7551#discussion_r1684809866
##########
java/java.hints/src/org/netbeans/modules/java/hints/introduce/IntroduceMethodPanel.java:
##########
@@ -254,6 +253,7 @@ private Preferences getPreferences() {
public void setOkButton( JButton btn ) {
this.btnOk = btn;
+ changeSupport.run();
Review Comment:
so i debugged this and the whole panel initialization runs off-EDT(!), this call included ;)
What I did though to not make it worse: I removed this call to `run()` again, and triggered validation when the default value is set.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Introduce-method refactoring should suggest a method name [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #7551:
URL: https://github.com/apache/netbeans/pull/7551#issuecomment-2241941645
I think I addressed all review comments, if there is more I should do or if I overlooked something please say so. Planning to merge this soon since I don't want to wait till last minute before freeze.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Introduce-method refactoring should suggest a method name [netbeans]
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on PR #7551:
URL: https://github.com/apache/netbeans/pull/7551#issuecomment-2217393881
> however it could create the situation that pressing enter would do nothing the first time its pressed
If the OK button is disabled, then pressing `ENTER` _should_ do nothing. No issue with prefilling with a valid value. But the user should still need to use `ESC` to cancel the dialog.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Introduce-method refactoring should suggest a method name [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #7551:
URL: https://github.com/apache/netbeans/pull/7551#issuecomment-2217491412
> If the OK button is disabled, then pressing ENTER should do nothing.
the problem is that OK is disabled. You type something and it won't apply. The fact that cancel happens to be selected is just a followup issue. I can set `btnCancel.setDefaultCapable(false)` but this won't really change anything on the original problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Introduce-method refactoring should suggest a method name [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #7551:
URL: https://github.com/apache/netbeans/pull/7551#discussion_r1684825968
##########
java/java.hints/src/org/netbeans/modules/java/hints/introduce/IntroduceMethodPanel.java:
##########
@@ -254,6 +253,7 @@ private Preferences getPreferences() {
public void setOkButton( JButton btn ) {
this.btnOk = btn;
+ changeSupport.run();
Review Comment:
well. this creates another problem i wanted to avoid. Now I probably going to have to init `valid` with true, otherwise it would appear to users which press enter quickly as if the keyboard isn't working.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Introduce-method refactoring should suggest a method name [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #7551:
URL: https://github.com/apache/netbeans/pull/7551#discussion_r1684825968
##########
java/java.hints/src/org/netbeans/modules/java/hints/introduce/IntroduceMethodPanel.java:
##########
@@ -254,6 +253,7 @@ private Preferences getPreferences() {
public void setOkButton( JButton btn ) {
this.btnOk = btn;
+ changeSupport.run();
Review Comment:
well. this creates another problem i wanted to avoid. Now I probably going to have to init `valid` with true.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists