This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
| Summary: | Fast Import (Alt Shift I) should trim selected text | ||
|---|---|---|---|
| Product: | editor | Reporter: | sergionalin <sergionalin> |
| Component: | -- Other -- | Assignee: | issues@editor <issues> |
| Status: | RESOLVED FIXED | ||
| Severity: | blocker | CC: | jkovar, mihmax |
| Priority: | P3 | Keywords: | REGRESSION, SIMPLEFIX |
| Version: | 3.x | ||
| Hardware: | All | ||
| OS: | Windows XP | ||
| Issue Type: | ENHANCEMENT | Exception Reporter: | |
| Attachments: |
Patch to be applied on main trunk
Patch to be applied on release341 branch Better patch (no more null exceptions) Proposed patch for trunk |
||
|
Description
sergionalin
2002-09-02 17:54:37 UTC
Enchancment, will not be fixed in 3.4.1 just to let you know, see also: http://www.netbeans.org/devhome/docs/releases/34/ Sincere, Maxym Sorry, I didn't clarify myself: This issue is considered as a candidate for 3.4.1, but it's not a "MUST HAVE" bugfix (that's why I removed it blocking issue 28441). Sorry Created attachment 8017 [details]
Patch to be applied on main trunk
The patch I have attached is a diff from the main trunk. It basically modify just one file (JavaFastImport.java) trimming the text. The patch is compatible with issue #25497, which is not marked as CANDIDATE_3.4.1. Integrating the fix without issue #25497 requires a slightly different patch. I remain available to provide this if needed and when needed. :-) Yes, Gabriele, please make a patch without issue 25497 - I'm not sure it can/may go into 3.4.1 I'll make a patch to the 3.4.1 trunk. There is one thing, though: there are two other issues (#26063 and #26430) that have been solved after issue #25497, and are 3.4.1_CANDIDATE. If I make the patch to the 3.4.1 trunk, it means they have to make patches for those after mine since I don't see them integrate in 3.4.1. Is it ok? Or is it better that I wait for the integration of #26063 and #26430 (since this issue is not really a MUST_HAVE)? (Ok, maybe I am being too cautious... It's the first time I contribute to such a big project. ;-) ) If it does not matter that the #25497 was an enhancement I would integrate them all. But IMHO Maxym should finally decide. OK, so this enchancement is small enough to be OK even the bugfix release (3.4.1 is that). Please, post to nbreviewers about all the issues (2 or 3), I hope noone objects these RFEs 2 Gabriele: Thanks for the diff, _both_ fixes should be integrated into 3.4.1, so there's no need in other .diff 2 Gabrielle again ;-) Bugfixes for issue 26063 and issue 26430 are both to go into 3.4.1 Hi. This issue is marked as 3.4.1_CANDIDATE. It means that it should be integrated into release341 one branch. The plan at http://www.netbeans.org/devhome/docs/releases/34/index.html expected beta1 to be produced on Dec01. That did not happen due to a lot of outstanding not integrated candidates like this one. Would it be possible to spend few minutes by backporting this fix? Thank you in advance. 2 Gabriele: #25497 is not going to 3.4.1, so please provide a different patch. The one you provided really ;-) doesn't work. I will provide a patch to the release341 branch. When you say the patch didn't work, do you mean to the release341 branch (which I know) or to the trunk (which should work)? :-) in release341 ;-) of course I haven't look at trunk since starting 3.4.1 initiative Created attachment 8204 [details]
Patch to be applied on release341 branch
I attached the patch. Sorry it took so long, I needed some quiet time to figure out the extra binary stuff... No problem it took long, Frinay night build - a possible 3.4.1 beta wasn't done anyway ;-( Thanks a lot BTW: Since the patch is So Simple ;), I commit it right away: Checking in editor/libsrc/org/netbeans/editor/ext/java/JavaFastImport.java; new revision: 1.7.24.2; previous revision: 1.7.24.1 done Sorry folks, but Gabriele's patch is mailfunctioning, actually it
throughs NPE every time I'm between two spaces: " | " and press
Alt+Shift+I.
In this piece of code:
exp = exp.trim();
exp is null if we're not immediately after of before the identifier,
so trimming it gives us nothing.
Checking in
editor/libsrc/org/netbeans/editor/ext/java/JavaFastImport.java;
new revision: 1.7.24.3; previous revision: 1.7.24.2
2Gabriele: null is returned, I suppose, by "Utilities.getSelectionOrIdentifier(target);" method, so the trimming must be done somewhere in Utilities class, not in JavaFastImport class. Sincere, Maxym Hi Maxym, I am sorry: I feel I am making you lose time over something very trivial. I don't think the trim should be done in the Utility class, since the function has to return the selection, and the selection might contain spaces. I'll just check if null is returned, and don't trim. I'll try to test the fix better before attaching it this time. Created attachment 8210 [details]
Better patch (no more null exceptions)
Well, Gabriele, the patch you provided is useless if (| being caret) you have the following situation and press Alt+Shift+I: "RemoteException | re;" I this situation the "exp" is null, so trimming doesn't help much ;-) I mean, what's the testcase where this trimming DOES help? I saw none. Sincer, Max Maxym, the trim is useful when you have selected something, as the summary suggests. ;-) If you select "RemoteException ", without the patch you don't import anything, with it you do. I tend to select (that's why I didn't notice the problem with the patch), you seem not to, so you'll never notice the difference. :-) The behaviour without selection was intended to stay the same. If you ask: why should I select "RemoteException " in the first place? It comes out when you use Ctrl+right to select a word. Yes, it _is_ something very very very tiny... ;-) but it gets annoying. And every little counts. Hope it's clear now. :-) OK, accepted. Small feature, and now that we know all the BUGs ;-), it's ready to be applyed. Checking in editor/libsrc/org/netbeans/editor/ext/java/JavaFastImport.java; new revision: 1.7.24.4; previous revision: 1.7.24.3 done Editor guys! please commit to trunk Thanks Wait before integrating to trunk! :-) Due to issue #25497, I have to provide a different patch. Created attachment 8412 [details]
Proposed patch for trunk
Sorry I took so long. The fix is compatible with other issues that wheren't included in 3.4.1, in particular #25497 and #26761. The patch doesn't seem to have been committed to the trunk. And I actually noticed that the file it is patching has changed... What happens now? :-) This feature was included in 3.4.1 but has disappeared in the released 3.5. Can it please be reinstated? Well, I would be willing to provide another patch. Question is: will this time somebody have a look at it and integrate it? :-( Gabriele, thank you for the patch. I have added you to NetBeans credit file in core/release/CREDITS.html I have used your carcassi@netbeans.org email, if you would like to change it, please let me know. the bug has been fixed in [maintrunk] /cvs/editor/libsrc/org/netbeans/editor/ext/java/JavaFastImport.java,v <-- JavaFastImport.java new revision: 1.14; previous revision: 1.13 also implemented in [prj40_prototype] /cvs/java/editor/libsrc/org/netbeans/editor/ext/java/Attic/JavaFastImport.java,v <-- JavaFastImport.java new revision: 1.1.2.1.2.1.2.3; previous revision: 1.1.2.1.2.1.2.2 Thanks again, Mato PS: we are sorry for omitting this issue for a long time ;-) *** Issue 34707 has been marked as a duplicate of this issue. *** CCing our sustainer... Martin, I suppose you marked it a DEFECT by mistake, or do I miss anything? BTW, since this is a trivial patch, and was present in 3.4.1 and not in 3.5, shouldn't we mark it both as SIMPLEFIX and REGRESSION? :-) OK, I've added both requested keywords. It's a regression against 3.4.1, so we should include the fix in the next update of 3.5. |