Navigation between the calltips for overloaded functions

[darmar]

Currently the calltips for overloaded functions are displayed on separate calltip pages. To navigate between the calltip pages the keyboard shortcuts "Ctrl+N" and "Ctrl+P"  are used.

I think the use of these shortcuts are not intuitive. In VisualStudio and MonoDevelop the simple "ArrowUp" and ArrowDown" are used instead. I find it is much more intuitive and corresponds to how the user navigates in the code-completion list.

I tried to implement the required changes into C::B. Now I have the working version. If developers have interest, I can prepare a patch.

[oBFusCATed]

I have serious interest, I've raised this question long ago and it has not been addressed.
So you patch will be very welcome and I'll test it right away.

[darmar]

Here is the patch.
This patch adds changes to the scintilla. I guess, this is not very desirable, but I haven't found another way.
Note: this patch doesn't delete Menu items "Next Call Tip" and "Previous Call Tip". I could prepare a patch for it if required.

I am ready to make adjustments in the patch if required.

[oBFusCATed]

Thank you. Can you post the changes to Scintilla to the maintainers of Scintilla?
They can integrated them in the official release or they can tell you how to improve them or do it in a better way if needed.

[darmar]

I have updated the patch. Now the change in Scintilla was avoided.

[Alpha]

I had been trying to achieve this for a while; I shall try to review your patch soon.

[Alpha]

Committed, thank you.  I did change it slightly so that pressing down/up when there is no next/previous entry will instead cancel the tip, moving the caret to the next/previous line.

[teto]

Great feature ! Much appreciated, thanks darmar

[oBFusCATed]

I've just tried it and I'm not sure I like it.
Is it possible to make it wrap-around?

[oBFusCATed]

FYI: The tooltips in VStudio 2010 are wrapping around, too.

[darmar]

There is no technical problems to make the call-tips wrap-around. It just depends on the taste of developers.

[oBFusCATed]

Yes, I know, I've just tried it is way better and logical.

[oBFusCATed]

Here is the patch I'm testing:

Code

commit ecd133cdf16bd7e493c9b59deb363036e67d75a5
Author: T Petrov <tpetrov@codeblocks.org>
Date:   Thu Jan 29 08:33:57 2015 +0200

    * ccmanager: Make the call tips wrap around
    
    > Introduce a helper function that advances to the previous or next call tip.
    > It goes to the first if the user wants to go to the next tip and it is at
      the last tip.
    > It goest to the last tip if at the first and going backwards.
    > Use it everywhere to do the advancing of tips (key handling, menu
      handling and call tip, button presses).
    > Always show both forward and backward buttons in the calltip, so the user
      will know that we support wraparound.

diff --git a/src/include/ccmanager.h b/src/include/ccmanager.h
index fd7a435..8cd89fa 100644
--- a/src/include/ccmanager.h
+++ b/src/include/ccmanager.h
@@ -76,6 +76,7 @@ class DLLIMPORT CCManager : public Mgr<CCManager>, wxEvtHandler
         void DoHidePopup();
         void DoShowDocumentation(cbEditor* ed);
         void DoUpdateCallTip(cbEditor* ed);
+        void AdvanceTip(bool next);
         /** format tips by breaking long lines at (hopefully) logical places */
         void DoShowTips(const wxStringVec& tips, cbStyledTextCtrl* stc, int pos, int argsPos, int hlStart, int hlEnd);
 
diff --git a/src/sdk/ccmanager.cpp b/src/sdk/ccmanager.cpp
index 6ac2f13..5e2ba7b 100644
--- a/src/sdk/ccmanager.cpp
+++ b/src/sdk/ccmanager.cpp
@@ -397,6 +397,28 @@ void CCManager::InjectAutoCompShow(int lenEntered, const wxString& itemList)
     }
 }
 
+// Change the current call tip to be the next or the previous.
+// Do wrapping if the end is reached in both directions.
+void CCManager::AdvanceTip(bool next)
+{
+    if (next)
+    {
+        ++m_CurCallTip;
+        if (m_CurCallTip == m_CallTips.end())
+            m_CurCallTip = m_CallTips.begin();
+    }
+    else
+    {
+        if (m_CurCallTip == m_CallTips.begin())
+        {
+            if (m_CallTips.size() > 1)
+                m_CurCallTip = m_CallTips.begin() + m_CallTips.size() - 1;
+        }
+        else
+            --m_CurCallTip;
+    }
+}
+
 bool CCManager::ProcessArrow(int key)
 {
     bool wasProcessed = false;
@@ -406,12 +428,13 @@ bool CCManager::ProcessArrow(int key)
     cbStyledTextCtrl* stc = ed->GetControl();
     if (stc->CallTipActive() && m_CallTipActive != wxSCI_INVALID_POSITION && m_CallTips.size() > 1)
     {
-        if (key == WXK_DOWN && (m_CurCallTip + 1) != m_CallTips.end())
-            ++m_CurCallTip;
-        else if (key == WXK_UP && m_CurCallTip != m_CallTips.begin())
-            --m_CurCallTip;
+        if (key == WXK_DOWN)
+            AdvanceTip(true);
+        else if (key == WXK_UP)
+            AdvanceTip(false);
         else
-            return wasProcessed; // moved off end, cancel tip
+            return wasProcessed;
+
         DoUpdateCallTip(ed);
         wasProcessed = true;
     }
@@ -814,12 +837,12 @@ void CCManager::OnEditorHook(cbEditor* ed, wxScintillaEvent& event)
         switch (event.GetPosition())
         {
             case 1: // up
-                --m_CurCallTip;
+                AdvanceTip(false);
                 DoUpdateCallTip(ed);
                 break;
 
             case 2: // down
-                ++m_CurCallTip;
+                AdvanceTip(true);
                 DoUpdateCallTip(ed);
                 break;
 
@@ -1073,16 +1096,12 @@ void CCManager::OnMenuSelect(wxCommandEvent& event)
         return;
     if (event.GetId() == idCallTipNext)
     {
-        if ((m_CurCallTip + 1) == m_CallTips.end())
-            return;
-        ++m_CurCallTip;
+        AdvanceTip(true);
         DoUpdateCallTip(ed);
     }
     else if (event.GetId() == idCallTipPrevious)
     {
-        if (m_CurCallTip == m_CallTips.begin())
-            return;
-        --m_CurCallTip;
+        AdvanceTip(false);
         DoUpdateCallTip(ed);
     }
 }
@@ -1184,16 +1203,9 @@ void CCManager::DoUpdateCallTip(cbEditor* ed)
     cbStyledTextCtrl* stc = ed->GetControl();
     if (m_CallTips.size() > 1)
     {
-        ++offset;
-        if (m_CurCallTip == m_CallTips.begin())
-            tips.front().Prepend(wxT('\002')); // down arrow
-        else if (m_CurCallTip + 1 == m_CallTips.end())
-            tips.front().Prepend(wxT('\001')); // up arrow
-        else
-        {
-            tips.front().Prepend(wxT("\001\002")); // up/down arrows
-            ++offset;
-        }
+        tips.front().Prepend(wxT("\001\002")); // up/down arrows
+        offset += 2;
+
         tips.push_back(wxString::Format(wxT("(%d/%u)"), m_CurCallTip - m_CallTips.begin() + 1, m_CallTips.size()));
         // store for better first choice later
         m_CallTipChoiceDict[CCManagerHelper::CallTipToInt(m_CallTips.front().tip, m_CallTips.size())] = m_CurCallTip - m_CallTips.begin();

Any feedback is welcome. If there is none, I'll push it as is! 

[Alpha]

Any feedback is welcome. If there is none, I'll push it as is! 

I am fine with this.  The only thing I would mention is I do not like bool parameters since if one just reads AdvanceTip(false); it is not clear exactly what it does without looking up the definition.  An enum would be better (either create one, or use a wx one).

[oBFusCATed]

OK, In svn...