Move bookmarks on changes to line numbers
Fixes #137 @aaxu /cc
Note that I am working on refactoring on Bookmarks so that it will end up as a separate class. However, thus far, we have been using a tuple. In python, when comparing tuples, I think it compares the first element and if those are equal, it compares the second element, and so on. Using this information, you can use bisect_left/bisect_right to get to where you want.
Are you essentially trying to get the list of bookmarks on the screen? If so, you can take a look at getVisibleBookmarks in window.py to see how it works.
My idea was to mock the __get__ methods etc. and running bisect_left on that.
This functions needs to shift all bookmarks below the lines that changed.
Maybe we can somehow defer some of that using getVisibleBookmarks but could be fragile.
In order to do that, could you use bisect to find the first bookmark you need to change, (should be the first one after the cursor) and iterate the bookmark list starting from there?
Yeah, I think that should be doable. And when bookmarks have their own class, we can perform only the changes currently visible on the screen and store a list of the pending moves.
Ohh nice idea!
Think so, too. If everything goes well, we can even upgrade to a r-tree :laughing: which we can use to efficiently keep track of which line intervals are affected by which changes.
If you know the lower limit, wouldn't you be able to use bisect as well and find the last bookmark closest to that limit?
Okay, I admit it's a bit hacky, but it works just fine. It's using a anonymous object providing:
__getitem__ = lambda x, index: self.bookmarks[index][0][1]__len__ = lambda x: len(self.bookmarks)
Thus, finding the first bookmark whose lower limit is >= upper
Performance should be fine, deferring using a r-tree or similar should wait on the refactoring by @aaxu. The behavior is as expected, but there are some edge cases where we might wanna change it. Any further thoughts @dschuyler?
Maybe this isn't so much of a bug as a design decision, but if you are inside a bookmark and you create new lines, those new lines will also be part of the bookmark. This can lead to a very long bookmark, and may cause the user to have to delete/recreate it again.
Yeah, I thought this was the point of multi-line bookmarks. So, what is the desired behavior? Splitting the bookmark into two?
I think this makes sense if we are modifying a 2+line bookmark, but I thought it would be better in the case of having a single line bookmark to maybe keep it as a single bookmark (maybe keep it it at the beginning in the case where the line breaks). I don't use bookmarks, so I can't really say but I personally don't like a bookmark indicator of large size, though I will defer the decision to @dschuyler since he may have a stronger opinion.
Also, upon some more testing, I found that if you create a bookmark and then 'move' a bookmark by deleting a line before it, jumping to next bookmark (F2) will go to the old position of the bookmark, and jumping to previous bookmark (SHIFT_F2) will not work anymore.
Hmm, you have a point regarding single line bookmarks. Maybe we could differentiate between "newline" at the beginning of the first line, at the end of the last line (could be same as first line) and somewhere in between? But I am curious, what would you do as a user if you wanted to extend it?
jumping to next bookmark (F2) will go to the old position of the bookmark
Really? Are these values held elsewhere?
Yes I was thinking of that as well. Something along the following lines for single-line bookmarks:
- If newline at beginning of bookmark, shift entire bookmark down
- if newline at end of a bookmark, do nothing
Also, I'm not sure what I would do as a user if I wanted to extend it. Extending it honestly doesn't have any functionality other than a visual marker, since jumping to a bookmark will place the cursor at the beginning of the bookmark. I think it may have something to do with where the bookmark would be placed as well on the screen once we jump to it.
If we are extending multiline bookmarks, we will also need to handle how the selection mode saved also changes (maybe just change it to selectionNone for simplicity?)
I think the bug is caused by bookmarkData not being changed. Even though you are changing the bookmark position, the bookmarkData still contains old positions. These positions are used to move the cursor back to its original position (from when the bookmark was created), so they should be changed too.
Also, I'm not sure what I would do as a user if I wanted to extend it.
How about ctrl+e -> bm++ or something?
Extending it honestly doesn't have any functionality other than a visual marker
Extending single-line to multi-line or having multi-line at all? I would certainly miss multi-line bookmark markers.
Just realized: don't we need to update these values on changes to individual lines as well?
Extending bookmarks in general seems to only serve as a visual indicator. I don't see it as having any other functionality that's different from if we didn't extend the bookmark. For example, if you had two bookmarks of different length, jumping to either of them will position them such that the beginning of the bookmark is near the top of the screen. The only difference is if a bookmark is longer than the height of the screen, the beginning of the bookmark will then be situated at the top of the screen when we jump to it.
And yes, we would need to update the data of the bookmarks if we 'move' them regardless of whether they are single-line or multilined.
My last comment was meant differently. I meant changes in the content of a line: "dolor # sit amet" -> "lorem #ipsum dolor sit amet" "dolor # sit amet" -> "lorem ipsum dolor # sit amet" # stands for either of the positional markers.
Oh yes, you're right. We would need to discuss how to handle these cases. I did consider using positional markers before, but then we'd have to adjust bookmarks for every insertion/deletion. That could end up giving very high overhead, so it may be better to take a more simplistic approach.. This question also extends further to what we should do if a bookmark corresponds to a block of selected text and how we would adjust those values as well.
I would like @dschuyler 's opinion on this since I have no experience using bookmarks so I don't completely understand whether marking the general position is more important, or keeping selection/position of the cursor to where it was before. I would imagine, however, that keeping track of a selection would be useful as a reminder if you want to revisit a bookmark, but easily forget why you bookmarked that spot.
It should help making these values relative the the bookmark position, if possible.
Thanks for working through this.
Is it time to step back and consider what we want from bookmarks?
I imagine that users may have different needs for bookmarks
- to mark a spot to come back to
- might just be one point (not a range)
- might just be one of them
- not much need to store the one bookmark
- can be updated (when editing), but probably unnecessary
- to jump between two (or three) locations temporarily
- this is what I use them for personally
- not much need to store them
- akin to putting one's fingers in different sections of a book to flip back and forth
- can be updated (when editing), but probably unnecessary
- to make a note for (personal) future reference (like a future editing session)
- these would need to be stored
- akin to bookmarks in a web browser (not URLs per se, but bookmarks)
- can be updated (when editing) and maybe should be
- to share (with another person) a reference to a location
- these could be stored (but not critical)
- akin to URLs
- may help to be ascii text (rather than binary data)
- impractical to update
- to link locations between documents
- akin to a web page link
- the storage would be in the linking documents
- impractical to update
I think I initially implemented # 2 (per my needs at the time). It sounds like we're heading to # 3 (which I'm okay with).
I'm flexible on whether a bookmark should select a range.
Maybe bookmarks should refer to a 'thing' (which I can see working with # 4 or # 5), like a
- word
- line
- point
- class
- function
- paragraph
- comment
- relative range
- regex
- file
i.e. if the bookmark is something like a URL, then part of the encoding could be the type of thing referred to. So if the bookmark was for the class FooBar in the file my_stuff.py, then maybe we could open file my_stuff.py and located the class definition for FooBar and go there (regardless of which line it is on).
Making bookmarks refer to "things" would not allow them to be domain agnostic. Obviously, this can lead to issues if the file content gets modified with another editor. But for the time our editor is used for editing, we can have bookmarks "sticking" to text.
Maybe we should consider differentiating between "point" bookmarks (caret positions) and "range" bookmarks (line intervals). This should be known on creation and would sort out edge cases where we would otherwise not know how things should behave. They should mostly correspond to cases 1, 2, 5 and 3, 4 respectively.
I was still under the impression that bookmarks was for #2 as well. It's just that sometimes I tend to jump between multiple sections and it would be something like a 'temporary note' (not saved for future sessions).
I would maybe use only single point bookmarks rather than a highlighted range as a bookmark. I'm not sure on how often used this range bookmark would be used, but I would fine to discard it altogether and stick with single line bookmarks to reduce the complexity of this feature.
For example, instead of how we bookmark multiple lines if multiple lines are selected, we would bookmark only the first selected line, and perhaps save the cursor position on that line?
Have we yet figured out how to treat the edge cases?