Patch #3376 2012-11-23 16:39
alpha0010
GotoMatchingBrace: ensure nearby lines are visible- Download
- 3376-GotoMatchingBr.patch (1.5 KB)
Index: src/sdk/cbeditor.cpp
===================================================================
--- src/sdk/cbeditor.cpp (revision 8732)
+++ src/sdk/cbeditor.cpp (working copy)
@@ -375,6 +394,15 @@
return url;
}
+ void BufferPosition(cbStyledTextCtrl* stc)
+ {
+ const int dist = stc->VisibleFromDocLine(stc->GetCurrentLine()) - stc->GetFirstVisibleLine();
+ if (dist >= 0 && dist < 2)
+ stc->LineScroll(0, dist - 2);
+ else if (dist >= stc->LinesOnScreen() - 2)
+ stc->LineScroll(0, 3 + dist - stc->LinesOnScreen());
+ }
+
// vars
bool m_strip_trailing_spaces;
bool m_ensure_final_line_end;
@@ -2255,7 +2295,10 @@
int newLine = p_Control->FindChangedLine(fromLine, toLine);
if (newLine != wxSCI_INVALID_POSITION)
+ {
p_Control->GotoLine(newLine);
+ m_pData->BufferPosition(p_Control);
+ }
}
void cbEditor::GotoPreviousChanged()
@@ -2271,7 +2314,10 @@
int newLine = p_Control->FindChangedLine(fromLine, toLine);
if (newLine != wxSCI_INVALID_POSITION)
+ {
p_Control->GotoLine(newLine);
+ m_pData->BufferPosition(p_Control);
+ }
}
void cbEditor::SetChangeCollection(bool collectChange)
@@ -2438,8 +2484,11 @@
// now, we either found it or not
if (matchingBrace != wxSCI_INVALID_POSITION)
{
+ // move to the actual position
control->GotoPos(matchingBrace);
control->ChooseCaretX();
+ // make nearby lines visible
+ m_pData->BufferPosition(control);
}
}
History
The current version of the patch is not acceptable, because you'll break navigation type plugins like browsetracker.
The better option is to use EnsureVisible instead of GotoLine.
Sorry, my summary of this patch is poor. It has nothing to do with (un)folding lines (in fact, I completely forgot about that).
What it does is, if the caret jumps to a place is outside of the editor, it scrolls a little extra, so the next line is visible (ex: make the "if (...)" visible on the line before a "{" when Ctrl-Shift-B (goto matching brace) was pressed on a "}").
According to the documentation I saw on the wiki, there is nothing in this patch that should conflict with navigation plugins. However, I do not regularly use BrowseTracker; do you have a test case I could work with?
No steps to reproduce, because this is just a guess.
Anyway the correct way is to use EnsureVisible. Redo the patch using this call and I'll try it. Until then it is rejected!
wxScintilla::EnsureVisible() does _not_ scroll the editor, it only unfolds blocks (when necessary).
Here is an alternate way, using scroll calculations instead of wxScintilla::GotoLine(). Would this be preferred?
Have you looked at this: http://www.scintilla.org/ScintillaDoc.html#SCI_SETYCARETPOLICY
At first glance, I've not found something that might help, but I've not read it thoroughly...
Thanks for the link; I will read to see if I can find anything of use.
From my reading of that page, I was unable to anything directly related to the functionality of this patch.
Are there any changes to the current implementation of this patch that you would recommend?
If you have no objections, I will move this to my queue soon.
1. no need to add it to the queue, when someone has assigned himself to the patch, a PM is enough
2. there are two/three changes in this patch, so a better commit message is needed
3. (most important) I see a lot of code duplication, please use a helper function in an anonymous namespace placed at the top of the file.
1. Sorry, sometimes I forget you are the same person (as your user-names are different).
2. Position jump: ensure nearby lines are visible (previous/next change; matching brace)
3. Done.
In svn, a bit modified...
Next you can think how to make it a bit smarter...
Like showing the next/prev empty line. Think about the case where you have a function def with many parameters spanning multiple lines.