Bug 257893

The Caret API is located in editor.lib2 module in
org.netbeans.api.editor.caret
org.netbeans.spi.editor.caret
packages.
The architecture description and javadocs contain more detailed information about API/SPI usage.

TCR MM01: We need to add tests for the API/SPI methods.

Please review the Caret API on branch editor_mutli_caret in jet-main[1]. Thanks!

[1] - http://hg.netbeans.org/jet-main/branches

1) Could you please, generate diff file and attach it here?
2) Are you going to replace current In-place rename impls (SyncDocumentRegion copy pasted in quite a few places in NB) with the new API?

SD-1: EditorCaret.getCaretAt(int) not implemented ?

SD-2: EditorCaret.getDot/Mark could note for clarity that they relate to the last caret position (now the info is available in overview doc, compatibility note)

SD-3: CaretMoveContext.getOriginalSortedCarets() [other getSorted* methods] -- I could not find what is the sort order if some of the Positions are ShiftPositions - sort order is specified by dot, is the behaviour for carets on the same pos but different shift possible or defined ?

SD-4: EditorActionNames.toggleTypingMode possibly has wrong @since; Wrong @since in GapList.copy()

SD-5: will be there unit tests for EditorCaret API ?

SD-6: EditorCaret could expressly note the threading model/constraints; seems to allow calls from non-EDT, but possibly needs to be obtained from the EDT from the JTextComponent ?

SD-7: editor.lib2/apichanges.xml - invalid issue number in "shift-highlights-sequence" entry. editor.lib2 project.xml defines editor.util spec = 1.63, it should be 1.64

impl: Please check locking between EditorCaret.runTransaction and getCarets():
* caretInfos lazy init synchronized on listenerList,
* caretInfos invalidation NOT synchronized by the same variable, but a r/w lock.
=> if runTransaction successfully locks(), then caretInfos may enter the synchronized section even though lockThread != null.

Created attachment 158507 [details]
Diff of the Caret API

Diff of editor_multi_caret branch to trunk containing the Caret API.

(In reply to Vladimir Voskresensky from comment #3)
> 1) Could you please, generate diff file and attach it here?
Attached. I'm currently working on moving of the implementation classes into a separate package so I'll attach a new cleaner diff once that's done.

> 2) Are you going to replace current In-place rename impls
> (SyncDocumentRegion copy pasted in quite a few places in NB) with the new
> API?
Yes we are considering that but the SyncDocumentRegion is able to synchronize basically any document inserts and removals (e.g. from code completions items etc.) so it's more powerful than the carets that depend on related actions implementations. But in longterm a unification is desirable.

(In reply to Svata Dedic from comment #4)
> SD-1: EditorCaret.getCaretAt(int) not implemented ?
Yes, it will be implemented by binary searching the sorted caret infos. I did not implement it (IIRC there's a single view hierarchy usage of this method).

> 
> SD-2: EditorCaret.getDot/Mark could note for clarity that they relate to the
> last caret position (now the info is available in overview doc,
> compatibility note)
Fixed.

> 
> SD-3: CaretMoveContext.getOriginalSortedCarets() [other getSorted* methods]
> -- I could not find what is the sort order if some of the Positions are
> ShiftPositions - sort order is specified by dot, is the behaviour for carets
> on the same pos but different shift possible or defined ?
Yes, it will be ShiftPositions aware order.
Our longterm intent is to replace the rectangular selection with the multi-caret approach so the corresponding actions will all be shift-positions aware i.e. if a caret stands "inside" a tab character e.g. on a third "space" a subsequent insert will replace the tab character by inserting two spaces followed by the typed char.

> 
> SD-4: EditorActionNames.toggleTypingMode possibly has wrong @since; Wrong
> @since in GapList.copy()
Fixed, thanks.

> 
> SD-5: will be there unit tests for EditorCaret API ?
Definitely, it's already my MM01 TCR :-)

> 
> SD-6: EditorCaret could expressly note the threading model/constraints;
> seems to allow calls from non-EDT, but possibly needs to be obtained from
> the EDT from the JTextComponent ?
Yes, we do not want to be too restrictive but in reality almost all usages are from actions that are typically called in EDT. We could add checks for EDT and see the non-conforming callers and decide then whether to insist on EDT or not.

> 
> SD-7: editor.lib2/apichanges.xml - invalid issue number in
> "shift-highlights-sequence" entry. editor.lib2 project.xml defines
> editor.util spec = 1.63, it should be 1.64
Fixed, thanks.

> 
> impl: Please check locking between EditorCaret.runTransaction and
> getCarets():
> * caretInfos lazy init synchronized on listenerList,
> * caretInfos invalidation NOT synchronized by the same variable, but a r/w
> lock.
> => if runTransaction successfully locks(), then caretInfos may enter the
> synchronized section even though lockThread != null.
Fixed, thanks, the null-ing of caretInfos had to be within "synchronized (listenerList) { ... }" above in runTransaction().

The changes seem fine to me. I'm not unfortunately able to provide super-thorough review, as the diff is quite huge, but the new API is OK and I haven't found any issues.

Thanks for the review.

Revert to original title "Introduce Caret API"