Bug 217163 - There is no way to define deleted word interceptor/behavior
There is no way to define deleted word interceptor/behavior
Status: RESOLVED FIXED
Product: editor
Classification: Unclassified
Component: CSL (API & infrastructure)
7.3
All All
: P3 (vote)
: 7.4
Assigned To: Milutin Kristofic
issues@editor
: API_REVIEW
Depends on:
Blocks: 222506
  Show dependency treegraph
 
Reported: 2012-08-21 13:07 UTC by Martin Fousek
Modified: 2013-04-18 02:20 UTC (History)
8 users (show)

See Also:
Issue Type: DEFECT
:


Attachments
Patch for DeleteWord API (223.41 KB, patch)
2012-11-06 15:52 UTC, Milutin Kristofic
Details | Diff
CamelCase API (106.83 KB, patch)
2013-04-09 13:07 UTC, Milutin Kristofic
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Fousek 2012-08-21 13:07:27 UTC
In the CSL infrastructure was used KeystrokeHandler#getNextWordOffset() to determine words offsets so you was able to control deletion of words (CTRL+Backspace). There is no such possibility to do it by using new typinghooks API so far.

Actually it could be probably workarounded by creation of the custom editor action "BaseKit.removePreviousWordAction" but in that case I'm not able to override and register such a type of action (or better to say, any action) in the CslEditorKit since it's not supported there at all - see also #204616.

So the best would be to have interceptor for this editor action as well. Fallback could be to provide way how to override default CslEditorKit's actions but it's probably not proper solution in times of CSL clean up as we discussed. Thanks...
Comment 1 Martin Fousek 2012-08-28 12:55:24 UTC
Sorry for increased the priority but it's really necessary for us to be able to implement delete word functionality. Any resolution like override of CslEditorKit actions or another interceptor is OK for us. Thanks.
Comment 2 Milutin Kristofic 2012-11-06 15:52:30 UTC
Created attachment 127236 [details]
Patch for DeleteWord API
Comment 3 Milutin Kristofic 2012-11-06 16:06:11 UTC
MK1: I introduced new API org.netbeans.modules.csl.api.DeleteWordHandler - DeleteWordHandler with Typing Hooks is the newer alternative for KeystrokeHandler. 

MK2: Example for use is in javascript2.editor/src/org/netbeans/modules/javascript2/editor/JsDeleteWordHandler.java There isn't used KeystrokeHandler

MK3: I rewrite all csl typing actions to default csl typing hooks. These typing hooks works only when language register KeystrokeHandler and are introduced just for backcompatibility. I tested it on html.editor, php.editor, web.core.syntax(jsp)

MK4: There is a test infrastracture in csl.api/test/unit/src/org/netbeans/modules/csl/editor/TypingCompletion.java .A few examples are in html.editor, php.editor, web.core.syntax. Clients of Csl are welcomed to use it. 

MK5: Html typing actions and Jsp typing actions are deleted. For Jsp Java typing hooks are working just well. And since Jsp had dependency on Java typing actions, I could finally deleted Java typing actions and Java BraceCompletion.
Comment 4 Ondrej Brejla 2012-11-08 13:58:27 UTC
OB01: Please, try to run org.netbeans.modules.php.editor.indent.PHPBracketCompleter.java test with these changes applied. 
Probably you should run tests of all clients ;)

OB02: And I'm not sure if such a big change should be implemented after Beta2...isn't that too big?

Thanks.
Comment 5 Ondrej Brejla 2012-11-08 14:21:25 UTC
Ad OB01: 8 tests failed, please look at it. Thanks.


OB03: DeleteWordHandler.getNextWordOffset...returns int, so there shouldn't be @CheckForNull annotation imho

OB04: same for int getPreviousWordOffset(Document doc, int caretOffset);, probably no @CheckForNull
Comment 6 Martin Fousek 2012-11-08 14:22:38 UTC
Thanks for implementation in the JS, JSP. I didn't try it out them in practice, I'll take a look on that tomorrow.

API:
MF01 - Improve DeleteWordHandler class javadoc olease. At least registration HowTo would be welcomed there.
MF02 - Deprecated KeystrokeHandler but not usages in the API - like UiUtils#getBracketCompletion()? So basically does it mean that the API will have another review in the future?

Clients:
MF03 - Shouldn't you update spec. version dependencies of the JSP, HTML and PHP on the CSL? It looks to me that one can update i.e. html module but the CSL is not updated together so he will lose functionality?
MF04 - Clean-up unused imports and trailing spaces of all classes please (including API).
MF05 - Has the JsDeleteWordPositionHandler in the JS2 layer any purpose?

OB01: +1
Comment 7 Ondrej Brejla 2012-11-08 14:35:29 UTC
OB05: Some files are changed just because of added/removed whitespaces (e.g. Language). Don't include em in the patch please :) Thanks a lot.
Comment 8 Martin Janicek 2012-11-08 15:02:03 UTC
Thanks for doing this Mita!

MJ01: I would suggest to create only one method with a boolean reverse
parameter instead of having two methods. Don't see much benefits from it (most
of the clients will do exactly the same as you did in JsDeleteWordHandler -
create private method with boolean parameter to remove duplicity). Or is there
any particular reason for that?

MJ02: Would it be possible to move the DeleteWordHandler to Editor Library 2
which contains all other interceptors? Not sure if it's possible, but seems to
me a little bit confusing that we have two similar concepts (intercepting key
events) splitted in two different modules

MJ03: Minority problem - could you please clean up the patch a little bit?
There are some classes (e.g. JspKit) where you are adding new import
statements, but they are never used.

One more note to MF05: If it does have any purpose, there is a wrong link to
the DeleteWordPositionHandler which is not in csl.api (I guess it just wasn't
refactored correctly)
Comment 9 Milutin Kristofic 2012-11-13 15:54:58 UTC
@MK5 Removing JSP, Java Actions in Bug #219834

I am rewriting whole API. I am implementing DeleteWordInterceptor to editor.lib2. I am adding a replaced text variable for TypedTextIntereptor, it will help to pass php tests.
Comment 10 Martin Fousek 2012-11-21 07:43:27 UTC
Since it looks like the new API will not be available in the NB7.3 I used to old way using KeystrokeHandler (web-main #0fc25bfe7604) in the javascript2 for now.

BTW, this API still really make sense to me since our keystrokeHanlder has implemented only one method needed in cases of the word offsets.

So feel free to set TM => Next. Thanks...
Comment 11 Milutin Kristofic 2012-11-21 14:29:23 UTC
Martin, thank you. I am sorry, I had a lot of work recently and these changes are quite big.
Comment 12 Milutin Kristofic 2013-04-09 13:07:13 UTC
I prepared new API. I added new interceptor for camel case action like remove next/previous word, select next/previous word and insert point to next/previous word.

I moved java and csl action to editor.actions and implement csl and java implementation of camel case interceptor.

The client of csl has now 3 options:
1, implement keystrokehandler
2, implement keystrokehandler only with getNextWordOffset method and implement typing hooks without camelcase
3, implement all typing hooks (also with camelcase) and sets typing hook to null.

I checked php editor test and all passed after this change. Php uses now 2nd option.
Comment 13 Milutin Kristofic 2013-04-09 13:07:49 UTC
Created attachment 133390 [details]
CamelCase API
Comment 14 Martin Fousek 2013-04-15 07:37:47 UTC
Sorry, I'm not able to review whole diff since it's pretty big change in areas I don't know well. Anyway I tried at least implement CamelCaseInterceptor for JavaScript which worked well to me and I was able to remove deprecated KeystrokeHandler then.

MF06: Since the KeystrokeHandler is divided into 3-4 interceptors, it would probably deserve better JavaDoc how to switch from deprecated interface.
Comment 15 Milutin Kristofic 2013-04-16 12:57:48 UTC
Martin, thank you for a test and comment.

Integrated in http://hg.netbeans.org/jet-main/rev/90e449c21ae3
Comment 16 Quality Engineering 2013-04-18 02:20:24 UTC
Integrated into 'main-golden', will be available in build *201304172301* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/90e449c21ae3
User: Milutin Kristofic <mkristofic@netbeans.org>
Log: #217163 - There is no way to define deleted word interceptor/behavior


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo