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...

[dmoore]

OK, In svn...

Just updated to this version on my main dev system, and I have to say that this feature is sometime quite annoying. e.g. I am editing a bunch of function calls to take an additional argument, then I try to arrow down to the next instance and I can't without pressing escape first.

Ctrl+Up/Down would be better IMO.

[oBFusCATed]

Ctrl+Up/Down would be better IMO.

The problem is that we cannot do much about this. And shortcuts handling in C::B is pretty bad (partly because wx is bad/limiting).
Probably you can add an option to disable the arrow handling and then use the keybinder to set the shortcuts as you like them.

[Alpha]

There is a "single page only" option you could enable as an immediate workaround to your issue.  ... Though, there must be a better logic set that could be applied.