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

Set to history_list.py restore the viewport position instead of always focusing on the selections[0] #2964

Closed
evandrocoan opened this issue Sep 1, 2019 · 3 comments

Comments

@evandrocoan
Copy link

evandrocoan commented Sep 1, 2019

Description

This issue is an immediate consequence of jump_back and jump_forward does not record most actions #2959 As now, every change to the selections is creating a new history entry, running jump_back after using find_under_expand a few times, always focus on the first selection, instead of the latest selection focused by find_under_expand:

        self.view.window().focus_view(view)
        view.sel().clear()
        view.sel().add_all(region_list)
        view.show(region_list[0], True)

By also restoring the viewport position, we can always correctly focus on the latest selection from find_under_expand. However, if the buffer changed its size, (file reloading), then all saved viewport positions would be unsynchronized with the new buffer size.

Then, a suggestion would be to add support for add_regions to update saved regions and also update the saved viewport position, when the buffer changes.

Suggested patch for restoring the viewport position instead of running view.show(selections[0])

--- a/history_list.py
+++ b/history_list.py
@@ -43,7 +43,7 @@ class JumpHistory():
 
         # check the newest entry is not the same as selection
         if self.history_list != []:
-            first_view, first_key = self.history_list[-1]
+            first_view, first_key, viewport = self.history_list[-1]
             if first_view.view_id == view.view_id:
                 first_sel = view.get_regions(first_key)
                 if first_sel == region_list:
@@ -53,7 +53,7 @@ class JumpHistory():
         view.add_regions(key, region_list)
 
         # set the new selection as the current item, as a tuple (view_id, key)
-        self.history_list.append((view, key))
+        self.history_list.append((view, key, view.viewport_position()))
         self.trim_selections()
 
     def jump_back(self, active_view):
@@ -73,24 +73,24 @@ class JumpHistory():
 
         if self.current_item == -len(self.history_list):
             # already pointing to the oldest
-            return None, []
+            return None, [], None
 
         # get the next (older) selection
         self.current_item -= 1
-        view, key = self.history_list[self.current_item]
-        return view, view.get_regions(key)
+        view, key, viewport = self.history_list[self.current_item]
+        return view, view.get_regions(key), viewport
 
     def jump_forward(self, active_view):
         if self.history_list == []:
-            return None, []
+            return None, [], None
         # already pointing to the front
         if self.current_item >= -1:
-            return None, []
+            return None, [], None
         # get the top selection
         # print(self.current_item)
         self.current_item += 1
-        view, key = self.history_list[self.current_item]
-        return view, view.get_regions(key)
+        view, key, viewport = self.history_list[self.current_item]
+        return view, view.get_regions(key), viewport
 
     def remove_view(self, view_id):
         i = 0
@@ -118,7 +118,7 @@ class JumpHistory():
 
         # remove all history that are newer than current
         for i in range(-1, self.current_item):
-            view, key = self.history_list[i]
+            view, key, viewport = self.history_list[i]
             view.erase_regions(key)
         del self.history_list[self.current_item + 1:]
         # set current_item to the imaginary back (current caret position not yet pushed)
@@ -130,7 +130,7 @@ class JumpHistory():
             # trim to a smaller size to avoid doing on this every insert
             for i in range(0, len(self.history_list) - self.LIST_TRIMMED_SIZE):
                 # erase the regions from view
-                view, key = self.history_list[i]
+                view, key, viewport = self.history_list[i]
                 view.erase_regions(key)
             del self.history_list[: len(self.history_list) - self.LIST_TRIMMED_SIZE]
 
@@ -237,7 +237,7 @@ class JumpBackCommand(sublime_plugin.TextCommand):
         # get the new selection
         jump_history = get_jump_history_for_view(self.view)
 
-        view, region_list = jump_history.jump_back(self.view)
+        view, region_list, viewport = jump_history.jump_back(self.view)
         if region_list == []:
             sublime.status_message("Already at the earliest position")
             return
@@ -250,7 +250,10 @@ class JumpBackCommand(sublime_plugin.TextCommand):
         self.view.window().focus_view(view)
         view.sel().clear()
         view.sel().add_all(region_list)
-        view.show(region_list[0], True)
+        # print( region_list )
+        # view.show(region_list[0], True)
+        view.set_viewport_position( viewport, True )
+
         sublime.status_message("")
         unlock_jump_history()
 
@@ -267,7 +270,7 @@ class JumpForwardCommand(sublime_plugin.TextCommand):
         # jump back in history
         # get the new selection
         jump_history = get_jump_history_for_view(self.view)
-        view, region_list = jump_history.jump_forward(self.view)
+        view, region_list, viewport = jump_history.jump_forward(self.view)
         if region_list == []:
             sublime.status_message("Already at the newest position")
             return
@@ -281,7 +284,8 @@ class JumpForwardCommand(sublime_plugin.TextCommand):
         view.sel().clear()
         view.sel().add_all(region_list)
         # print(region_list)
-        view.show(region_list[0], True)
+        # view.show(region_list[0], True)
+        view.set_viewport_position( viewport, True )
         sublime.status_message("")
 
         unlock_jump_history()

Environment

  • Operating system and version:
    • Windows 10 build 15063 x64
    • Resolution 1920x1080
    • dpi_scale 1.0
    • Sublime Text:
    • Build 3207
    • 64 bit

Related Threads

  1. jump_back and jump_forward does not record most actions jump_back and jump_forward does not record most actions #2959
  2. jump_prev jumps too far jump_prev jumps too far #1424
  3. Jump to the last caret/cursor, on the current file Jump to the last caret/cursor, on the current file #1374
  4. Jump forward doesn't work on Linux Jump forward doesn't work on Linux #861
  5. Jumping back and forth to the latest history entry, erases all histories entries on the next new history entry addition Jumping back and forth to the latest history entry, erases all histories entries on the next new history entry addition #2962
  6. Increase jump history size from 100 to 1000 and stop using add_regions for tracking history Increase jump history size from 100 to 1000 and stop using add_regions for tracking history #2963
  7. "Show unsaved changes" panel scrolled to bottom "Show unsaved changes" panel scrolled to bottom #2504
  8. Go to line does not center the view correctly for the first time when the panel size is greater than half of the screen Go to line does not center the view correctly for the first time when the panel size is greater than half of the screen #2506
  9. Restore the output.exec panel view cursor and scroll position after building a project Restore the output.exec panel view cursor and scroll position after building a project #2547
  10. Pasting text on full selected view scroll everything past end with the option scroll_past_end Pasting text on full selected view scroll everything past end with the option scroll_past_end #1974
  11. Restore scroll position for all open files when restoring the workspace Restore scroll position for all open files when restoring the workspace #454
  12. Minimap and scroll_past_end are "zooming" but not scrooling Minimap and scroll_past_end are "zooming" but not scrooling #2814
  13. Restore the output.exec panel view cursor and scroll position after building a project Restore the output.exec panel view cursor and scroll position after building a project #2547
  14. If a not focused view file is reloaded, its scroll position is reset near to the beginning of its file If a not focused view file is reloaded, its scroll position is reset near to the beginning of its file #2473
  15. The output build panel is completely scrolled horizontally to the right when there are build errors The output build panel is completely scrolled horizontally to the right when there are build errors #2239
  16. Closing or changing a project before fully loading files/views, loses the last cursor/caret and scroll position Closing or changing a project before fully loading files/views, loses the last cursor/caret and scroll position #1740
  17. "Expand Selection to Word" is Tied to Search and Identical to "Quick Add Next" "Expand Selection to Word" is Tied to Search and Identical to "Quick Add Next" #2282
  18. ST3 find_under_expand does not respect find settings ST3 find_under_expand does not respect find settings #743
  19. Calling Ctrl+K, Ctrl+D to skip item, does not skip the item and selects wrong items Calling Ctrl+K, Ctrl+D to skip item, does not skip the item and selects wrong items #2091
  20. find_under_expand_skip resets whole-word state of find_under_expand find_under_expand_skip resets whole-word state of find_under_expand #1113
  21. When I am focused on the output.exec panel, and press Ctrl+D, the find_under_expand, is performed on the unfocused view When I am focused on the output.exec panel, and press Ctrl+D, the find_under_expand, is performed on the unfocused view #1894
  22. "find_under_expand" (ctrl+d) command has no effect with words containing certain unicode chars "find_under_expand" (ctrl+d) command has no effect with words containing certain unicode chars #1737
  23. Ctrl-D doesn't highlight first match when used after Ctrl-F Ctrl-D doesn't highlight first match when used after Ctrl-F #2931
  24. Ctrl+d (Multiple Selection) when the find panel is open messes up. (bug) Ctrl+d (Multiple Selection) when the find panel is open messes up. (bug) #797
@FichteFoll
Copy link
Collaborator

FichteFoll commented Sep 2, 2019

I don't see a need for this, personally. When I jump back to where I was, I'm perfectly fine with having that region in the center of my view where it gets my immediate attention rather than cycling through old viewport positions that a) might have changed through edits and b) I don't remember anyway.

That said, I do seldomly use these commands.

Also, a gigantic 24-item "related" list doesn't exactly help because the signal to noise ratio is too low. The only directly related issue I could extract from your issue description seems to be #2959. All the others are maginally related at best and sometimes only because they also use View.show internally or just involve viewports.

@evandrocoan
Copy link
Author

I don't see a need for this, personally. When I jump back to where I was

With the current implementation for history_input, there will be no problem as you describe. But after add the #2959 feature, it will not work right as now because there will be multiple selections when jumping back/forth, and always focusing on the first selection, will hide the actual last selection when they spread across several pages.

at best and sometimes only because they also use View.show internally or just involve viewport

Yeah, as I suggested:

Then, a suggestion would be to add support for add_regions to update saved regions and also update the saved viewport position, when the buffer changes.

These bugs are related to the view centering implementation.

@wbond
Copy link
Member

wbond commented Aug 13, 2020

This won't work, since it won't deal with modifications to the buffer between jumping around. That is the reason we use add_regions() instead of tracking the selection in a Python object, like was proposed in #2963.

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

No branches or pull requests

3 participants