Please use the Apache issue tracking system for new NetBeans issues (https://issues.apache.org/jira/projects/NETBEANS0/issues) !!
Bug 257893 - Introduce Caret API
Introduce Caret API
Status: NEW
Product: editor
Classification: Unclassified
Component: -- Other --
-FFJ-
All All
: P1 (vote)
: 8.2
Assigned To: Miloslav Metelka
issues@editor
: API, API_REVIEW
Depends on: 257888 257889
Blocks: 200027 258824
  Show dependency treegraph
 
Reported: 2016-02-08 14:22 UTC by Ralph Ruijs
Modified: 2016-05-23 19:20 UTC (History)
6 users (show)

See Also:
Issue Type: ENHANCEMENT
:


Attachments
Diff of the Caret API (578.85 KB, patch)
2016-02-15 14:10 UTC, Miloslav Metelka
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ralph Ruijs 2016-02-08 14:22:41 UTC
Introduce a Caret API to allow working with multiple carets within one document.
Comment 1 Miloslav Metelka 2016-02-09 15:11:51 UTC
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.
Comment 2 Ralph Ruijs 2016-02-10 07:30:19 UTC
Please review the Caret API on branch editor_mutli_caret in jet-main[1]. Thanks!

[1] - http://hg.netbeans.org/jet-main/branches
Comment 3 Vladimir Voskresensky 2016-02-11 14:51:09 UTC
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?
Comment 4 Svata Dedic 2016-02-15 10:01:15 UTC
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.
Comment 5 Miloslav Metelka 2016-02-15 14:10:34 UTC
Created attachment 158507 [details]
Diff of the Caret API

Diff of editor_multi_caret branch to trunk containing the Caret API.
Comment 6 Miloslav Metelka 2016-02-15 14:17:27 UTC
(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.
Comment 7 Miloslav Metelka 2016-02-16 09:05:22 UTC
(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().
Comment 8 Jaroslav Havlin 2016-02-23 14:30:20 UTC
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.
Comment 9 Miloslav Metelka 2016-03-08 14:17:59 UTC
Thanks for the review.
Comment 10 markiewb 2016-05-23 19:20:05 UTC
Revert to original title "Introduce Caret API"


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