Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for multiple carets and selection ranges #222

Closed
ghost opened this issue Dec 5, 2015 · 19 comments · Fixed by #687
Closed

Add support for multiple carets and selection ranges #222

ghost opened this issue Dec 5, 2015 · 19 comments · Fixed by #687

Comments

@ghost
Copy link

ghost commented Dec 5, 2015

Hi Tomas, is it on the radar to support multiple caret positions and selection ranges(with the ability to insert,paste text into those rsnges)? Just noticing that Flowless, and RichTextFX are going through major refactoring, so thought of asking :)

@JordanMartinez
Copy link
Contributor

I wonder if it might be wise to implement #152 before doing this...

@ghost
Copy link
Author

ghost commented Dec 5, 2015

You guys know better!

@ghost
Copy link
Author

ghost commented Dec 5, 2015

This can enable many things like refactoring code variables, (members?), paste into multiple ranges , etc

@JordanMartinez
Copy link
Contributor

Let me explain my thinking:
When removing the skin, StyledTextArea's dispose method was no longer needed, but it will be needed once 152 is implemented. So, since we've already done a lot of refactoring as whole, why not continue it with that part as well? Then, any new changes would be built on top of this refined implementation of the MVC paradigm.

However, I might be saying this because I'm viewing the master branch in terms of a "story" rather than "how it was developed" (I recently read an article that explained people usually view a git history in terms of one of those two ideas, but I can't remember where that article is).

@TomasMikula
Copy link
Member

I agree with Jordan that continuing the refactoring is more a natural immediate course, because, let's clean up the basics first. That doesn't mean that multiple carets/selections can't be implemented first. But unless someone steps up and does the work (:wink:), it will not happen before.

@JordanMartinez
Copy link
Contributor

True... except I'm currently focused on one of the last problems in my program (and then I can release it.... at least, once I figure out how to do that :-D)

At the same time, 152 isn't a high priority right now, just a nice thing to have in my specific use case.

Would you be up for implementing 152 @melkhaldi ? I don't have a need to implement multiple carets or selections as of right now, and Tomas seems to be working on other things. Yes, there would be a higher chance of getting this feature implemented if you did it, but my guess is that this feature relates enough to 152 that there would be reason for you to do that as well. At the same time, you would probably understand the library well enough to implement 152 if you implemented this feature, or vice versa, depending on the order taken.

@ghost
Copy link
Author

ghost commented Dec 6, 2015

Oh no! You really don't want me near your code. At my best I may be the
hacker type of user who fiddles with methods with lots of patience and
trial and error to find his way. I once hoped to try making a CodeField
(syntax highlighted text field) ...but that didn't really move... :/ I mean
I tried that a long time ago, but now I feel I got better at this gig! Will
try to go for a deep dive into the docs and see how the library is made
now, maybe something will unlock for me :). But you should soooooo not put
any eggs in my basket...
On Dec 5, 2015 6:10 PM, "JordanMartinez" [email protected] wrote:

True... except I'm currently focused on one of the last problems in my
program (and then I can release it.... at least, once I figure out how to
do that :-D)

At the same time, 152 isn't a high priority right now, just a nice thing
to have in my specific use case.

Would you be up for implementing 152 @melkhaldi
https://github.com/melkhaldi ? I don't have a need to implement
multiple carets or selections as of right now, and Tomas seems to be
working on other things. Yes, there would be a higher chance of getting
this feature implemented if you did it, but my guess is that this feature
relates enough to 152 that there would be reason for you to do that as
well. At the same time, you would probably understand the library well
enough to implement 152 if you implemented this feature, or vice versa,
depending on the order taken.


Reply to this email directly or view it on GitHub
#222 (comment)
.

@JordanMartinez
Copy link
Contributor

Umm.... Actually I want you near this code (Tomas probably does? I can't speak for him). Every time you fail, you'll learn something new. So see this as a learning experience.
Besides, if you've looked at all the PRs I've made throughout Tomas libraries, most of the time I get 80% (sometimes less ;-D) of it right, and then he helps me see the remaining 20% that still needs to be fixed.
I'm pretty sure Tomas appreciates it because features get implemented without needing too much of his time. It's easier for him to say "looks good" or "you forgot this here" and allow us to correct it (which allows us to learn) and get work done than it is for him to spend much of his time on something that, although he would like to have in his code, he doesn't need personally for his usage.

So, it's a sort of win-win situation. Everyone who uses this library benefits from the new features. We benefit from the experience and overall learning and confidence built. Tomas benefits in that he has more time to work on what he needs.

@ghost
Copy link
Author

ghost commented Dec 6, 2015

i am all for helping out... i am just not sure how helpful I can be, but
will give it a shot :D

On Sat, Dec 5, 2015 at 6:59 PM, JordanMartinez [email protected]
wrote:

Umm.... Actually I want you near this code (Tomas probably does? I can't
speak for him). Every time you fail, you'll learn something new. So see
this as a learning experience.
Besides, if you've looked at all the PRs I've made throughout Tomas
libraries, most of the time I get 80% (sometimes less ;-D) of it right, and
then he helps me see the remaining 20% that still needs to be fixed.
I'm pretty sure Tomas appreciates it because features get implemented
without needing too much of his time. It's easier for him to say "looks
good" or "you forgot this here" and allow us to correct it (which allows us
to learn) and get work done than it is for him to spend much of his time on
something that, although he would like to have in his code, he doesn't need
personally for his usage.

So, it's a sort of win-win situation. Everyone who uses this library
benefits from the new features. We benefit from the experience and overall
learning and confidence built. Tomas benefits in that he has more time to
work on what he needs.


Reply to this email directly or view it on GitHub
#222 (comment)
.

@JordanMartinez
Copy link
Contributor

Cool! 😃

@TomasMikula
Copy link
Member

@melkhaldi by all means, give it a shot! This one might not be the easiest one as a warm-up pull request, so don't despair if things don't work straight away!

@JordanMartinez
Copy link
Contributor

Hey @melkhaldi, can you give us an update?

  • What does the final outcome of the completed code look like?
  • What's your general idea of how to modify the code to get there?
  • What questions, if any, do you have?
  • What problems, if any, have you encountered where you are not sure about what should be done next? etc...

@ghost
Copy link
Author

ghost commented Dec 9, 2015

Hey Jordan, I haven't gotten around to it yet. I'll need to start wrapping
my head around the library's inner working .. I only used it as a user.
Will post once I have made progress . :/
On Dec 8, 2015 9:50 PM, "JordanMartinez" [email protected] wrote:

Hey @melkhaldi https://github.com/melkhaldi, can you give us an update?

  • What does the final outcome of the completed code look like?
  • What's your general idea of how to modify the code to get there?
  • What questions, if any, do you have?
  • What problems, if any, have you encountered where you are not sure
    about what should be done next? etc...


Reply to this email directly or view it on GitHub
#222 (comment)
.

@JordanMartinez
Copy link
Contributor

Just FYI (and in case you weren't aware) @melkhaldi, I've almost finished implementing #152 but there's still a minor bug to be worked out.

@JordanMartinez
Copy link
Contributor

Hey @melkhaldi, any updates? Or have you been busy with something else recently?

@ghost
Copy link
Author

ghost commented Jan 17, 2016

hey Jordan, no i haven't gotten around to it. I am not sure when though. things don't seem to taper off for the time being..

@JordanMartinez
Copy link
Contributor

Ok.

Having worked on the model-view modulation issue, I realize now how many things need to be taken into account:

CaretPosition has three parts to it:

  • The Var<Integer> internalCaretPosition used to position the caret
  • The SuspendableVal<Integer> caretPosition used to publish a valid caret position after changes have occurred.
  • The Val<Position> position that updates SuspendableVal<Integer> currentParagraph (its major) and SuspendableVal<Integer> caretColumn (its minor)

The Selection has five parts to it:

  • The Var<IndexRange> internalSelection used to track the start/finish of a selection
  • The selectionStart2D and selectionEnd2D used to track a selection position across multiple lines
  • The SuspendableVal<IndexRange> selection used to publish a valid selection after changes have occurred.
  • The SuspendableVal<String> selectedText used to publish a valid string of the currently selected text after changes have occurred.
  • The SuspendableVar<Integer> anchor

These two items should probably be turned into their own separate classes for modulation and easier management. To have multiple carets/selections, then, the model would use an ObservableList<Caret> or ObservableList<Selection>.

To actually be able to use it, one would need to account for the following things:

  • properly suspending each SuspendableVal/Var when an area changes the EditableStyledDocument
  • handling a plain/rich text change when multiple selections should be deleted/changed (e.g. renaming a method in a code editor). This would probably require a new class MultiTextChange whose subclasses PlainMultiTextChange and RichMultiTextChange would contain multiple TextChange objects within it. To apply the change, each of its underlying TextChange would be applied.
  • updating a clone's possible multiple carets and selections to still point to valid points/selections within the area
  • a switch/trigger of some sort that turns on the user's ability to use multiple carets/selections when the user does some specific interaction.
  • properly displaying multiple carets/selections throughout the area

@JordanMartinez
Copy link
Contributor

I think this should be distinguished from "support for multiple simultaneous changes". In other words, this specific issue should allow the option of multiple carets/selections throughout the area, but not yet handling multiple simultaneous changes (e.g. renaming a method). Rather, one could type something at Caret A's position and then type something at Caret B's position.

@JordanMartinez
Copy link
Contributor

I've implemented this in #687 and would appreciate any feedback on it before I merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants