Closed Acecool closed 2 months ago
mg979
commented
6 years ago It's a focus issue, I think the problem is here:
CodeMap synches the document after it has been saved, and focuses back on it, even if you changed to another one in the meantime. Personally I just delete those 2 lines (897-898).
Acecool
commented
6 years ago Cheers - but modifying core files isn't something I want to do with XCodeMapper... Hopefully oleg-shiro will correct this issue because it is detectable in code when you switch to a different file... I'll take a look at the file and see about writing a work-around and submitting it.
I've been trying for a while to figure out which addon was doing this and I was disabling 3 at a time and trying and finally I just tried CodeMap since I had no others left..
Thanks for the info :-) Also XCodeMapper just got an update - now scroll past end of file is disabled in the CodeMap panel and a new callback is created with the panel as an arg so other settings can be altered. I kept having issues with scroll past eof on CodeMap because it'd scroll to the bottom sometimes, but I needed scroll past eof enabled on ordinary files because of the issue with the status bar, and find panels sometimes covering up most of the last line...
oleg-shilo
commented
6 years ago Hi guys,
Sorry for not responding quickly on your requests. I am completely flooded out on Notepad++ plugin project.
The problem is caused by the notorious lack of true view/panel on ST3. Thus the fake-panel (dedicated CodeMap buffer/tab) can only do so much.
@mg979 is correct. the behavior is caused by focus switching. Though it is not a defect but a deliberate functionality (work around for the ST3 limitation).
On saving CodeMap supposed to scroll itself and reposition the selection - "synch_map". Good. This can only be done when CodeMap has focus. Thus the plugin needs to set focus to CodeMap, scroll and then set it back to the text.
If we follow @mg979 advise and disable "synch_map" then... well, it will be out of synch. Not good but may be OK for some users.
I will experiment with this and see if anything can be done. Will also have a look at other scrolling problem you logged. But I will not be able to come back to this project for another 1-2 weeks.
mg979
commented
6 years ago One quick fix could be not to make on_post_save async, and maybe call a normal short timeout on synch_map if it's needed for some reason, so that this focus theft doesn't happen. On the other hand, maybe to focus on the map wouldn't be needed in the first place, most stuff (adding selections/moving viewport/editing the view etc) also works if the view doesn't have focus, but if the quick fix works there's no need to change anything else.
Acecool
commented
6 years ago I'll see what I can do... It does seem to be a true view - so we have a lot of options there.... What I'll do is incorporate some of my code ( where I look for the code map panel ) and adapt it to your code...
The goal will be to keep code-map syncing ( typo by the way ) on save, and on load / re-focus...
Logic: -- Default case If not switching then keep doing what it is doing else - ie if switched -- Default case / catch to avoid running logic for simplest logic If new doc isn't mapped ( custom mapper or otherwise ) then prevent sync system from running.. If new file has a mapper then run sync on the current doc instead of the last one...
This should be possible...
Note: I'm editing core files - this is something I wanted to avoid... For the time being I will include the modded files in my bitbucket repo ( until you merge the update )...
oleg-shilo
commented
6 years ago I'll see what I can do... It does seem to be a true view - so we have a lot of options there....
Sorry to be cynical, but it is not a "true view" but an ordinary text buffer. It is exactly what problem with ST# hosting model is.
The absence of strong API support for a non-text view lead us to the situation when a simple loosely coupled original solution already became a hard to control complicated state-machine. Add inability to debug (I mean truly debug) and you have a fragile system that requires high effort to troubleshoot/maintain.
Thus if you are planing to do anything then proceed with an extreme caution.
mg979
commented
6 years ago @Acecool it's just a tiny focus issue. It's not worth messing too much with it, risking to break stuff. Unless it's a tiny change as well. I think the options are either deleting those two lines, or something like this on line 897:
if is_code_map_visible() and sublime.active_window().active_view() == view:
This should only synch the view if the current view is the same as the one that has been saved.
If it doesn't work, something like this:
def on_post_save(self, view):
if ACTIVE: # map view is opened
sublime.set_timeout(lambda: refresh_map_for(view))
# synch_map brings map_view into focus so call it only
# if it is not hidden behind other views
if is_code_map_visible():
sublime.set_timeout(lambda: synch_map(view))
# CodeMap file has been loaded but it's currently inactive
elif get_code_map_view():
sublime.set_timeout_async(reactivate)This should make the block more or less synchronous.
oleg-shilo
commented
6 years ago if is_code_map_visible() and sublime.active_window().active_view() == view:Yep, this is something that I was also thinking about.
Acecool
commented
6 years ago It acts like a true view - the api calls all seem to work on it when you can isolate the view ( typically it is the last view; haven't seen where it is anything other than the last one but I added logic to check for that too )... I haven't looked at this yet - having some issues with medication not having decent efficacy likely because the patch isn't sticking properly so I need to find a new location to rotate or something...
I have noticed debugging can be annoying because the code is evaluated among other issues, but you do sometimes get decent error messages - most of the time it is trash so debugging becomes more tedious but if you don't need to restart frequently then it's easy to map out... simply comment out the recent changes, or half, and go through that process... Typically it is an easy to find issue such as missing + or ! used, or missing : - which is why I'm also adding support for that in the python mapper ( have some, but not missing + yet - may way until I add the rule system before I start adding more syntax error checks )....
Cheers for the suggestions... I'll probably end up looking at it in a bit today or on Friday.
Acecool
commented
6 years ago So I took an initial look at it...
Right now 2 timeouts are being called - this isn't necessary from what I can see... We know that go will be called first and then the lambda function second - so we can simply add the focus line to the end of go and all should be good...
To address checking if the file has changed, that's basic.. Since go function is declared within sync_map as synch_map function, we can simply declare last_current_file in the scope - ie above go and in synch_map as sync_map function... Then we grab the current file when the function is called again and compare them. If the file is different we return which prevents the window focus from being called and all of the other code...
The solution is quite simple - now I just need to look through the code to find where the data is set...
But, in short - this is the code ( tested and working )
def synch_map(v, give_back_focus=True):
## TODO: Find the var / class controlling this
## Set the last current file to the one which is currently open
last_current_file = 'code_map.py'
##
## Allow go to be called - but cancel it if the file switched...
##
def go():
## TODO: Find the var / class controlling this
## Set the current file ( after the timeout ) for a comparison..
current_file = 'code_map.py'
## If the current file isn't what the sync_map function was called with, then we do not allow sync_map to continue otherwise it'll cause rubberbanding back to a file we've lost interest in...
if ( current_file != last_current_file ):
return
map_view = get_code_map_view()
if map_view and map_view.size() > 0:
code_view_line, _ = v.rowcol(v.sel()[0].a)
prev_map_line = None
lines = map_view.lines(Region(0, map_view.size()))
entries = []
index = 0
for line in lines:
link = map_view.substr(line).split(':')[-1]
try:
entries.append((int(link), line))
except:
continue
entries.sort(key=lambda tup: tup[0])
for member_line_num, line in entries:
# added +1 so that it works for the first line of the function
if member_line_num > code_view_line + 1:
break
else:
prev_map_line = line
map_view.sel().clear()
if prev_map_line:
map_view.sel().add(prev_map_line)
map_view.show(prev_map_line.a)
map_view.window().focus_view(map_view)
## Make the give back focus a part of go because it uses timeout anyway...
if give_back_focus:
win().focus_view(v)
## sublime.set_timeout(lambda: win().focus_view(v), 10)
# apply a timeout to the whole function, add an additional timeout if it's
# necessary to focus back to the original view - Acecool: But only if we haven't switched to a different file...
sublime.set_timeout(go, 10)So instead of 2 timeouts - we call 1... give_back_focus, since it is declared in the scope of go via an argument passed to synch_map as sync_map function - we can check it in the go function without any problems. and since 2 timeouts aren't needed, we comment the code out for now to be removed later...
Now, just need to find the var for the current function - and if one isn't set then I need to set it... Then the rubberbanding issue will be solved.
To test: replace synch_map function with the above code. Alter last_current_file or current_file so they don't match and it won't switch.. Keep the same and they do...
Worst case, if a var isn't set I'd either create it, or process the views ( as I do when I look for the Code - Map panel ) in order to find the file code mapper is processing ( I don't believe 3 or 2 files can be active at a time either using the multi-view group but that will need to be tested... I may simply have to look at the active file and if there are multiple groups open then I can change the logic so I don't give focus but I allow the rest of the syncing to be called ( to scroll to the right place in the Code - Map panel... )
Acecool
commented
6 years ago And solved: I'll include this in my repo until included in this repo although it'll require manual edit because I'm not sure if User/CodeMap can be used to replace files... I'll test it out.
def synch_map(v, give_back_focus=True):
## Grabs the active view from the active window and reads the filename from it...
## Note: We can do it this way, or use the view passed to the function - but we need to do it the former for the go function because v may have changed... For now we'll use v to grab the current file...
## last_current_file = = sublime.active_window( ).active_view( ).file_name( )
last_current_file = v.file_name( )
last_window = sublime.active_window( )
## Allow go to be called - but cancel it if the file / window has switched or if the view / file has been closed...
def go():
## Since T time has passed - we relearn data that may have changed.. This includes looking at variables passed through our scope - v is the VIEW / file we're mapping... v could be None now so if it is, then there's no point in proceeding..
if ( v == None ):
return
## Grab the active window - again T time has passed so we need to relearn data that may have changed...
current_window = sublime.active_window( )
## If the active window has changed, prevent the previous window from refocusing... No need to proceed if the window has changed...
if ( current_window != last_window ):
return
## Set the current file ( after the timeout ) for a comparison.. We don't use v because this is a function called after T time has passed and v will still be the same... We need to grab the active window, the active view within it, and the file-name of that view... If they have changed, we don't resync it..
current_file = current_window.active_view( ).file_name( )
## If the current file isn't what the sync_map function was called with, then we do not allow sync_map to continue otherwise it'll cause rubberbanding back to a file we've lost interest in...
if ( current_file != last_current_file ):
return
map_view = get_code_map_view()
if map_view and map_view.size() > 0:
code_view_line, _ = v.rowcol(v.sel()[0].a)
prev_map_line = None
lines = map_view.lines(Region(0, map_view.size()))
entries = []
index = 0
for line in lines:
link = map_view.substr(line).split(':')[-1]
try:
entries.append((int(link), line))
except:
continue
entries.sort(key=lambda tup: tup[0])
for member_line_num, line in entries:
# added +1 so that it works for the first line of the function
if member_line_num > code_view_line + 1:
break
else:
prev_map_line = line
map_view.sel().clear()
if prev_map_line:
map_view.sel().add(prev_map_line)
map_view.show(prev_map_line.a)
map_view.window().focus_view(map_view)
## Make the give back focus a part of go because it uses timeout anyway...
if give_back_focus:
win().focus_view(v)
# apply a timeout to the whole function, add an additional timeout if it's
# necessary to focus back to the original view - Acecool: But only if we haven't switched to a different file...
sublime.set_timeout(go, 10)Basically - we use v for the last_current_map because it is the view passed to sync_map function so we can assume it'll be the file we're actively parsing... When go is called, T time has passed but v is still the same ( but it may also be None, which isn't checked for, but it should be so I've added that too... ) - so we can't use v to check for the file-name because it'll be identical as each file is a view is what appears to be how Sublime Text does things... So we grab the active window, the active view and the filename from that - if they have changed then we don't sync something we are no longer focused on...
As noted: This contains a check to see if v is None - any time you offset time, expect the data to change so that is a bug fix... otherwise a logical error may occur: None type doesn't contain sub *...
I did not add logical checks to determine whether or not the window has changed - the reason I left that out is because CodeMap can be active in multiple windows - right? And if the file-name is different then the view would cancel out - although if the filename is the same then that may cause problems... Editing that check in.... - ADDED..
Changelog:
That should be all...
Note: I could probably change the length of additional logic by simply comparing the VIEW_ID... VIEW_ID should be unique, even between windows ( but it may not - and since I'm not sure whether or not they change or reset between windows, I've opted for the other approach but depending how things go further along it may be something to look into because it'll enable us to remove the file-name comparison and the window check and simply change v == None to v != current_view ... We could actually get rid of the v == None check because if v is None then it'll be different from current_view anyway... so is streamlines the logic quite a bit )...
Acecool
commented
6 years ago Note: New issue found - If you replace only 1 file from CodeMap in Packages ie the code_map.py file then it still tries to load md.sublime-syntax from custom_languages causing an error... I'll create a new issue for that along with a solution ( if file doesn't exist then don't load it, check in User/ and in the default location if one specified ) although I may simply ignore it if it doesn't exist otherwise I'd have to supply them too - for now I'll do that..
Acecool
commented
6 years ago oleg-shilo
commented
6 years ago Thank you for taking care of it. Merged. Will be available in the very next release.
Acecool
commented
6 years ago Cheers!
Edit: Just let me know ( unless there is a changelog which automatically opens in Sublime Text ) when the release is live so I know when to remove the Packages/CodeMap/ folder from my repo...
I've tried disabling every addon and testing... Basically if you edit a file and switch to another, CodeMap will immediately re-open the file that was just saved in order to re-process it, I'm guessing...
This is a huge problem for managing large quantities of files, configuration, etc... Please add logic that if the current view or file-name has changed, to not reprocess file on save... and instead process the new file...
This will make things a lot easier!
SOLVED
And solved: I'll include this in my repo until included in this repo although it'll require manual edit because I'm not sure if User/CodeMap can be used to replace files... I'll test it out.
Basically - we use v for the last_current_map because it is the view passed to sync_map function so we can assume it'll be the file we're actively parsing... When go is called, T time has passed but v is still the same ( but it may also be None, which isn't checked for, but it should be so I've added that too... ) - so we can't use v to check for the file-name because it'll be identical as each file is a view is what appears to be how Sublime Text does things... So we grab the active window, the active view and the filename from that - if they have changed then we don't sync something we are no longer focused on...
As noted: This contains a check to see if v is None - any time you offset time, expect the data to change so that is a bug fix... otherwise a logical error may occur: None type doesn't contain sub *...
I did not add logical checks to determine whether or not the window has changed - the reason I left that out is because CodeMap can be active in multiple windows - right? And if the file-name is different then the view would cancel out - although if the filename is the same then that may cause problems... Editing that check in.... - ADDED..
Changelog:
That should be all...
Note: I could probably change the length of additional logic by simply comparing the VIEW_ID... VIEW_ID should be unique, even between windows ( but it may not - and since I'm not sure whether or not they change or reset between windows, I've opted for the other approach but depending how things go further along it may be something to look into because it'll enable us to remove the file-name comparison and the window check and simply change v == None to v != current_view ... We could actually get rid of the v == None check because if v is None then it'll be different from current_view anyway... so is streamlines the logic quite a bit )...