-
Notifications
You must be signed in to change notification settings - Fork 38
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
Results keyboard navigation, keyboard shortcuts and fixes #67
base: master
Are you sure you want to change the base?
Changes from 5 commits
943ad3b
184ea07
e5e94d9
12e0562
b9f9a1c
580142c
c59dc98
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,6 +51,50 @@ | |
"command": "cscope_visiter", | ||
"context": [{"key": "selector", "operand": "text.find-in-files"}] | ||
}, | ||
{ | ||
"keys": ["j"], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I'd rather leave out the sublime-keymap changes at least for the first round and just make the changes that are required to support it. Can you remove these sublime-keymap changes for at least this first review and let's get through all the other changes first? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed |
||
"command": "cscope_visiter", | ||
"context": [ | ||
{"key": "selector", "operand": "text.find-in-files"}, | ||
{"key": "setting.cscope_results"} | ||
], | ||
"args": {"direction": "down"} | ||
}, | ||
{ | ||
"keys": ["k"], | ||
"command": "cscope_visiter", | ||
"context": [ | ||
{"key": "selector", "operand": "text.find-in-files"}, | ||
{"key": "setting.cscope_results"} | ||
], | ||
"args": {"direction": "up"} | ||
}, | ||
{ | ||
"keys": ["pageup"], | ||
"command": "cscope_visiter", | ||
"context": [ | ||
{"key": "selector", "operand": "text.find-in-files"}, | ||
{"key": "setting.cscope_results"} | ||
], | ||
"args": {"direction": "up2"} | ||
}, | ||
{ | ||
"keys": ["pagedown"], | ||
"command": "cscope_visiter", | ||
"context": [ | ||
{"key": "selector", "operand": "text.find-in-files"}, | ||
{"key": "setting.cscope_results"} | ||
], | ||
"args": {"direction": "down2"} | ||
}, | ||
{ "keys": ["ctrl+l", "ctrl+x"], | ||
"command": "cscope", | ||
"args": { "mode": "close_opened_results" } | ||
}, | ||
{ "keys": ["ctrl+l", "ctrl+f"], | ||
"command": "cscope", | ||
"args": { "mode": "database_rebuild" } | ||
}, | ||
{ | ||
"keys": ["ctrl+shift+["], | ||
"command": "goback" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,7 @@ | |
<array> | ||
<dict> | ||
<key>match</key> | ||
<string>(.*):$</string> | ||
<string>^(((?!scope: ).)*):$</string> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain why you changed this please? I'm not aware of a problem with this right now, so I'm not sure what this is trying to fix. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
<key>captures</key> | ||
<dict> | ||
<key>1</key> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,7 @@ | |
8: "files #including this file" | ||
} | ||
CSCOPE_CMD_DATABASE_REBUILD = "database_rebuild" | ||
CSCOPE_CMD_CLOSE_OPENED = "close_opened_results" | ||
|
||
def get_settings(): | ||
return sublime.load_settings("CscopeSublime.sublime-settings") | ||
|
@@ -41,6 +42,14 @@ def get_setting(key, default=None, view=None): | |
pass | ||
return get_settings().get(key, default) | ||
|
||
def update_result_selection(view, selection_index, selection, update_sel=True): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm confused on what this is doing here too. Can you please help me understand what this is doing that it wasn't already doing? Also, I suspect that this will not work with Sublime Text 2. Thus far, while I'm not actively supporting Sublime Text 2, I've been making sure I at least don't break it with changes. Can you verify that this will at least not cause errors in ST2? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I keep results and currently selected results in view settings of every Cscope results.
|
||
view.settings().set('cscope_results_sel', selection_index) | ||
view.add_regions('selection', [selection], 'comment', 'dot') | ||
view.show(selection) | ||
if update_sel: | ||
view.sel().clear() | ||
view.sel().add(sublime.Region(selection.a, selection.a)) | ||
|
||
|
||
class CscopeDatabase(object): | ||
def __init__(self, view, executable): | ||
|
@@ -124,11 +133,11 @@ def update_location(self, filename): | |
|
||
# If we haven't found an existing database location and root | ||
# directory, and the user has a project defined for this, and | ||
# there's only one path in that project, let's just assume that that | ||
# there's one or more paths in that project, let's just assume that first | ||
# path is the root of the project. This should be a safe assumption | ||
# and should allow us to create an initial database, if one doesn't | ||
# exist, in a sane place. | ||
if self.root is None and self.location is None and len(project_info_paths) == 1: | ||
if self.root is None and self.location is None and len(project_info_paths) >= 1: | ||
self.root = project_info_paths[0] | ||
print('CscopeDatabase: Database not found but setting root: {}' | ||
.format(self.root)) | ||
|
@@ -146,6 +155,14 @@ def rebuild(self): | |
if not (cscope_arg_list and isinstance(cscope_arg_list, list)): | ||
cscope_arg_list = [self.executable, '-Rbq'] | ||
|
||
# If the user has a project with more than one path and we're putting | ||
# the database in the first one, treat other paths as additional source dirs | ||
if get_setting('auto_next_source_dirs') and hasattr(self.view.window(), 'project_data'): | ||
project_info = self.view.window().project_data() | ||
if project_info and 'folders' in project_info and project_info['folders'][0]['path'] == self.root: | ||
for folder in project_info['folders'][1:]: | ||
cscope_arg_list += ["-s", folder['path']] | ||
|
||
print('CscopeDatabase: Rebuilding database in directory: {}, using command: {}' | ||
.format(self.root, cscope_arg_list)) | ||
|
||
|
@@ -170,10 +187,32 @@ class CscopeVisiter(sublime_plugin.TextCommand): | |
def __init__(self, view): | ||
self.view = view | ||
|
||
def run(self, edit): | ||
def run(self, edit, **args): | ||
if self.view.settings().get('syntax') == CSCOPE_SYNTAX_FILE: | ||
if args.get('direction'): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you help me understand what you're doing here? I gather the concept is to allow navigation in the results buffer using direction increments? Is this assuming there's only 1 cscope results window? I frequently have multiple cscope results windows. I'm not sure what the goal is here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, keyboard navigation. I also have multiple cscope results windows, no problems. Saved results and currently selected result are held in settings 'results' and 'cscope_results_sel' per cscope results buffer/view. |
||
d = args.get('direction') | ||
r = self.view.get_regions('results') | ||
if r == []: | ||
return | ||
view_height = len(r) - 1 | ||
s = self.view.settings().get('cscope_results_sel') | ||
what = { | ||
'up': -1, | ||
'down': 1, | ||
'up2': -20, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we are going to do this, I think maybe up/down/pageup/pagedown would be better than up/down/up2/down2 as far as names go. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree, changed |
||
'down2': 20 | ||
} | ||
if d not in what: | ||
return | ||
s = s + what[d] | ||
if s < 0: s = 0 | ||
if s > view_height: s = view_height | ||
|
||
update_result_selection(self.view, s, r[s]) | ||
return | ||
|
||
root_re = re.compile(r'In folder (.+)') | ||
filepath_re = re.compile(r'^(.+):$') | ||
filepath_re = re.compile(r'^(((?!scope: ).)+):$') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain this change please? Why the change in regexp? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If line with symbol ends with ':' it is catched by this regex (seen below), which leads to such error when trying to open results lower than that.
|
||
filename_re = re.compile(r'([a-zA-Z0-9_\-\.]+):') | ||
linenum_re = re.compile(r'^\s*([0-9]+)') | ||
|
||
|
@@ -295,24 +334,31 @@ def __init__(self, view, platform, database, symbol, mode, executable): | |
# switch statement for the different formatted output | ||
# of Cscope's matches. | ||
def append_match_string(self, match, command_mode, nested): | ||
match_string = "{0}".format(match["file"]) | ||
match_string = {} | ||
match_string['main'] = "{0}".format(match["file"]) | ||
if command_mode in [0, 4, 6, 8]: | ||
if nested: | ||
match_string = ("{0:>6}\n{1:>6} [scope: {2}] {3}").format("..", match["line"], match["scope"], match["instance"]) | ||
match_string['head'] = ("{0:>6}\n").format("..") | ||
match_string['main'] = ("{0:>6} [scope: {1}] {2}").format(match["line"], match["scope"], match["instance"]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, going to need help understand what you're doing here too, please, in each of these. I didn't write these regexps originally, so I honestly don't understand what they were doing completely nor what you're changing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The regexes just format results appearance. I only split them into the 'head' part with the separator ('..') or file path and 'main' part with proper result line, which was needed for keyboard navigation and selecting/highlighting. Effectively there is no difference in appearance. I would also add the option to the settings for disabling the separator between results altogether. |
||
else: | ||
match_string = ("\n{0}:\n{1:>6} [scope: {2}] {3}").format(match["file"].replace(self.database.root, "."), match["line"], match["scope"], match["instance"]) | ||
match_string['head'] = ("\n{0}:\n").format(match["file"].replace(self.database.root, ".")) | ||
match_string['main'] = ("{0:>6} [scope: {1}] {2}").format(match["line"], match["scope"], match["instance"]) | ||
elif command_mode == 1: | ||
if nested: | ||
match_string = ("{0:>6}\n{1:>6} {2}").format("..", match["line"], match["instance"]) | ||
match_string['head'] = ("{0:>6}\n").format("..") | ||
match_string['main'] = ("{0:>6} {1}").format(match["line"], match["instance"]) | ||
else: | ||
match_string = ("\n{0}:\n{1:>6} {2}").format(match["file"].replace(self.database.root, "."), match["line"], match["instance"]) | ||
match_string['head'] = ("\n{0}:\n").format(match["file"].replace(self.database.root, ".")) | ||
match_string['main'] = ("{0:>6} {1}").format(match["line"], match["instance"]) | ||
elif command_mode in [2, 3]: | ||
if nested: | ||
match_string = ("{0:>6}\n{1:>6} [function: {2}] {3}").format("..", match["line"], match["function"], match["instance"]) | ||
match_string['head'] = ("{0:>6}\n").format("..") | ||
match_string['main'] = ("{0:>6} [function: {1}] {2}").format(match["line"], match["function"], match["instance"]) | ||
else: | ||
match_string = ("\n{0}:\n{1:>6} [function: {2}] {3}").format(match["file"].replace(self.database.root, "."), match["line"], match["function"], match["instance"]) | ||
match_string['head'] = ("\n{0}:\n").format(match["file"].replace(self.database.root, ".")) | ||
match_string['main'] = ("{0:>6} [function: {1}] {2}").format(match["line"], match["function"], match["instance"]) | ||
elif command_mode == 7: | ||
match_string = ("\n{0}:").format(match["file"].replace(self.database.root, ".")) | ||
match_string['main'] = ("\n{0}:").format(match["file"].replace(self.database.root, ".")) | ||
|
||
return match_string | ||
|
||
|
@@ -395,9 +441,10 @@ def run_cscope(self, mode, word): | |
def run(self): | ||
matches = self.run_cscope(self.mode, self.symbol) | ||
self.num_matches = len(matches) | ||
self.output = "In folder " + self.database.root + \ | ||
header = "In folder " + self.database.root + \ | ||
"\nFound " + str(len(matches)) + " matches for " + CSCOPE_SEARCH_MODES[self.mode] + \ | ||
": " + self.symbol + "\n" + 50*"-" + "\n\n" + "\n".join(matches) | ||
": " + self.symbol + "\n" + 50*"-" + "\n" | ||
self.output = [ header, matches ] | ||
|
||
|
||
class CscopeCommand(sublime_plugin.TextCommand): | ||
|
@@ -505,8 +552,14 @@ def display_results(self, symbol, output): | |
|
||
cscope_view.set_syntax_file(CSCOPE_SYNTAX_FILE) | ||
cscope_view.set_read_only(True) | ||
cscope_view.settings().set('cscope_results', True) | ||
cscope_view.settings().set('cscope_results_sel', 0) | ||
|
||
def run(self, edit, mode): | ||
if mode == CSCOPE_CMD_CLOSE_OPENED: | ||
[x.close() for x in self.view.window().views() if x.settings().get('cscope_results')] | ||
return | ||
|
||
self.mode = mode | ||
self.executable = get_setting("executable", "cscope") | ||
|
||
|
@@ -571,7 +624,39 @@ def rebuild_database(self): | |
class DisplayCscopeResultsCommand(sublime_plugin.TextCommand): | ||
|
||
def run(self, edit): | ||
self.view.insert(edit, CscopeCommand.cscope_output_info['pos'], CscopeCommand.cscope_output_info['text']) | ||
r = [] | ||
header, matches = CscopeCommand.cscope_output_info['text'] | ||
self.view.insert(edit, CscopeCommand.cscope_output_info['pos'], header) | ||
for m in matches: | ||
head = '\n' + m['head'] | ||
start = self.view.size() | ||
self.view.insert(edit, start, head) | ||
start = self.view.size() | ||
self.view.insert(edit, start, m['main']) | ||
region = sublime.Region(start, self.view.size()) | ||
r.append(region) | ||
self.view.add_regions('results', r) | ||
if r: update_result_selection(self.view, 0, r[0]) | ||
if get_setting("display_outline") == True: | ||
symbol_regions = self.view.find_all(CscopeCommand.cscope_output_info['symbol'], sublime.LITERAL) | ||
self.view.add_regions('cscopesublime-outlines', symbol_regions[1:], "text.find-in-files", "", sublime.DRAW_OUTLINED) | ||
self.view.add_regions('cscopesublime-outlines', symbol_regions[1:], "entity.name.filename.find-in-files", "", sublime.DRAW_OUTLINED) | ||
|
||
class UpdateCscopeResultSelection(sublime_plugin.ViewEventListener): | ||
def __init__(self, *args, **kwargs): | ||
super(UpdateCscopeResultSelection, self).__init__(*args, **kwargs) | ||
self.mouse_point = -1 | ||
|
||
@classmethod | ||
def is_applicable(cls, settings): | ||
return settings.get('cscope_results') | ||
|
||
def on_selection_modified(self): | ||
mouse_point = self.view.sel()[0].a | ||
if mouse_point == self.mouse_point: | ||
return | ||
self.mouse_point = mouse_point | ||
r = self.view.get_regions('results') | ||
for x in r: | ||
if x.contains(mouse_point): | ||
update_result_selection(self.view, r.index(x), x, False) | ||
break |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious about the use case here. I've never been part of a project so big that it is split into multiple directories and is not contained within a common parent folder. Can you help me understand what kind of scenario would require this or how you have things set up such that this is useful?
Also, I wonder if we even need
auto_next_source_dirs
or if we can do the right thing automatically?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of this reason I didn't want to make it default option :) In my case the other folders contain library-like project (used by other 'main' projects also) or headers of this library, it is handy to have these sources in the same project. Sublime Text automatically indexes all folders in the project for definitions, so I guess this is similar behavior.
Also, it wasn't possible to update or create database with more than 1 folder in project, because of restriction
... len(project_info_paths) == 1