Function Completion menu item to cycle through choices #12017

vinsworldcom · 2022-08-15T18:52:05Z

Fix #11950

Add menu item in the Edit => Auto-Completion sub menu to cycle through overloaded function call tips.

This is the same menu spot where function call tips can be activated (Edit => Auto-Completion => Function Parameters Hint)
I do not re-order, simply add my option to the end, though a re-ordering would be nice to group the "function" related items together
I do not provide a language translation (english.xml) but it can be easily added with:
<Item id="50010" name="Function Parameters Next"/>
I only provide a "forward" / "next" (translates to a position=2 in Scintilla. This allows for overloads to go 1=>2=>3 .. =>n => 1, but not 3=>2=>1. Adding another menu item "Function Parameters Previous" would be trivial to call with position=1 but would add yet another menu item to just scroll through the choices in the opposite direction - I didn't think it was worth it.

Added to shortcut mapper, but shortcut.cpp needs a specific check for this command since its number (50010) is out of the current range due to other commands already using those IDs:

notepad-plus-plus/PowerEditor/src/resource.h

Lines 425 to 435 in f2c4ecd

    
           #define IDCMD 50000 
        
               //#define IDM_EDIT_AUTOCOMPLETE                (IDCMD+0) 
        
               //#define IDM_EDIT_AUTOCOMPLETE_CURRENTFILE    (IDCMD+1) 
        
           	#define IDC_PREV_DOC                    (IDCMD+3) 
        
           	#define IDC_NEXT_DOC                    (IDCMD+4) 
        
           	#define IDC_EDIT_TOGGLEMACRORECORDING    (IDCMD+5) 
        
               //#define IDC_KEY_HOME                    (IDCMD+6) 
        
               //#define IDC_KEY_END                        (IDCMD+7) 
        
               //#define IDC_KEY_SELECT_2_HOME            (IDCMD+8) 
        
               //#define IDC_KEY_SELECT_2_END            (IDCMD+9)

dfs- · 2022-08-16T09:00:16Z

PowerEditor/src/menuCmdID.h

    #define    IDM_EDIT_FUNCCALLTIP                             (50000 + 2)
    #define    IDM_EDIT_AUTOCOMPLETE_PATH                       (50000 + 6)
+    #define    IDM_EDIT_FUNCCALLTIP_NEXT                        (50000 + 10)


Wouldn't be 50000 + 3 more logical? The gap before IDM_EDIT_AUTOCOMPLETE_PATH was left intentional I guess.

50003 is already in use as is up to 50005. Up to 50009 are in, but commented. I wanted to avoid any issues:

notepad-plus-plus/PowerEditor/src/resource.h

Lines 425 to 435 in f2c4ecd

#define IDCMD 50000

//#define IDM_EDIT_AUTOCOMPLETE (IDCMD+0)

//#define IDM_EDIT_AUTOCOMPLETE_CURRENTFILE (IDCMD+1)

#define IDC_PREV_DOC (IDCMD+3)

#define IDC_NEXT_DOC (IDCMD+4)

#define IDC_EDIT_TOGGLEMACRORECORDING (IDCMD+5)

//#define IDC_KEY_HOME (IDCMD+6)

//#define IDC_KEY_END (IDCMD+7)

//#define IDC_KEY_SELECT_2_HOME (IDCMD+8)

//#define IDC_KEY_SELECT_2_END (IDCMD+9)

Cheers.

donho · 2022-08-22T17:58:51Z

@vinsworldcom
Could you provide the instructions to play and see the feature you've added (after pulling your PR)?

vinsworldcom · 2022-08-22T18:22:51Z

@donho happily! After this PR is incorporated, you can go to the Edit => Auto-Completion menu and see the new item "Function Parameters Next". I find it best to set a shortcut for this, so menu Settings => Shortcut Mapper... and filter on "function". I set the Alt+Down Arrow shortcut myself:

Next, open a new document and switch the language to Perl. Perl because I know the AutoCompletion file for Perl has some nice overloads to test with.

Type print( and you should see the calltip come up with indication that it is 1 out of 4 options.

Now, use the shortcut you created (in my case, Alt+Down Arrow to cycle through 2 of 4, 3 of 4, 4 of 4 and back to 1 of 4.

Hope this helps! It seems a pretty easy addition, the only "issue" I had was the menu item number "50010" since the others were taken and then not being able to just expand the else if() range in the 'shortcut.cpp' file, having to add the or statement. I've been running with this change since I made the PR and it's pretty nifty!

Cheers.

alankilborn · 2022-08-22T20:13:15Z

@vinsworldcom I have probably dumb questions:

How come there is only a provision for down and not up?
Why does this need a remappable shortcut key? When the completion box is visible, shouldn't down (or up) arrow presses change the current entry?

vinsworldcom · 2022-08-22T20:17:31Z

@alankilborn

How come there is only a provision for down and not up?

Typically, the Overloads start at 1 of # so you want to look at the next (rather than previous) and by looking at the next up to the last one and then one more, it rolls back over to 1. If it stopped, then certainly we'd need a "previous". I can add the previous, it just seemed like overkill to be able to scroll both directions for such a simple feature, but alas, users will probably ask for it. Should I add that?

Why does this need a remappable shortcut key? When the completion box is visible, shouldn't down (or up) arrow presses change the current entry?

That's exactly the issue described by the community post. You can use the mouse to click the arrows, but there is no way to use the keyboard. This PR - by assigning a shortcut to some keyboard combination - fixes that.

Cheers.

alankilborn · 2022-08-22T20:50:52Z

I can add the previous, it just seemed like overkill to be able to scroll both directions for such a simple feature, but alas, users will probably ask for it. Should I add that?

I would certainly leave that to your discretion. :-)

That's exactly the issue described by the community post. You can use the mouse to click the arrows, but there is no way to use the keyboard.

Right. But I think my point was, can't it be fixed by just handling "down arrow" (or "up arrow") keypresses when the completion box is visible? To my way of thinking, that would be what the user expects to happen. If the user has to remember a "special" keycombo (e.g. Alt+down) then it IMO kills the utility of it for all but the most die-hard of function completion keyboardists.

vinsworldcom · 2022-08-22T20:58:12Z

Right. But I think my point was, can't it be fixed by just handling "down arrow" (or "up arrow") keypresses when the completion box is visible? To my way of thinking, that would be what the user expects to happen. If the user has to remember a "special" keycombo (e.g. Alt+down) then it IMO kills the utility of it for all but the most die-hard of function completion keyboardists.

That's a good question. I'm sure it can, but it seems to involve a lot more coding than I know how to do. I think you'd need to hook the keyboard to intercept up/down arrow presses and check if a calltip is active, and if so, then send the SCN_CALLTIPCLICK notification. I also don't know how you'd handle a call tip without the overloads - in other words, nothing to scroll through. Scintilla let's you know a call tip is active SCN_CALLTIPACTIVE, but not whether it has overloads. So if it doesn't do we just intercept and "drop" the up/down arrow key presses, or send them on to the editor window, which of course scrolls the cursor and by default closes the calltip?

I don't see a callback handler for the CallTip functionality, which is where I think you'd put the checks for key presses of VK_UP / VK_DOWN.

I guess it would be similar to how the AutoComplete pulldown "captures" the up/down arrow keypresses. Again, I just don't see where that is in the code.

Cheers.

donho

Please add new entries into "english.xml" and consider the given suggestions.

donho · 2022-08-22T21:31:02Z

PowerEditor/src/Notepad_plus.rc

@@ -509,6 +509,7 @@ BEGIN
            MENUITEM "Word Completion",             IDM_EDIT_AUTOCOMPLETE_CURRENTFILE
            MENUITEM "Function Parameters Hint",    IDM_EDIT_FUNCCALLTIP
            MENUITEM "Path Completion",             IDM_EDIT_AUTOCOMPLETE_PATH
+            MENUITEM "Function Parameters Next",    IDM_EDIT_FUNCCALLTIP_NEXT


I would rename it as "Function Parameters Next Hint"

Put it just blow MENUITEM "Function Parameters Hint", IDM_EDIT_FUNCCALLTIP

Add a new "Function Parameters Previous Hint" between "Function Parameters Hint" & "Function Parameters Next Hint"

donho · 2022-08-22T21:39:17Z

PowerEditor/src/Notepad_plus.h

@@ -563,6 +563,7 @@ friend class FileManager;
 	void autoCompFromCurrentFile(bool autoInsert = true);
 	void showFunctionComp();
 	void showPathCompletion();
+	void showFunctionNext();


Since we have added new item "Function Parameters Previous Hint", the function prototype should be:
void showFunctionNextHint(bool isNext); // set isNext to false to reach the previous one

donho · 2022-08-22T21:41:40Z

PowerEditor/src/Parameters.cpp

@@ -149,6 +149,7 @@ static const WinMenuKeyDefinition winKeyDefs[] =
 	{ VK_SPACE,   IDM_EDIT_AUTOCOMPLETE_PATH,                   true,  true,  false, nullptr },
 	{ VK_RETURN,  IDM_EDIT_AUTOCOMPLETE_CURRENTFILE,            true,  false, false, nullptr },
 	{ VK_SPACE,   IDM_EDIT_FUNCCALLTIP,                         true,  false, true,  nullptr },
+	{ VK_NULL,    IDM_EDIT_FUNCCALLTIP_NEXT,                    false, false, false, nullptr },


I'm wondering if we should add Alt-Down & Alt-Up by default.
What do you people think?

I'm wondering if we should add Alt-Down & Alt-Up by default.
What do you people think?

I prefer no defaults and let people create their own. The other thought is @alankilborn 's comment about somehow making this "just work" with UP/DOWN arrows themselves when a calltip window is active, like UP/DOWN arrows just work on AutoComplete combo boxes when they appear. I don't know how / if this is possible, but maybe worth looking at?

Cheers.

@vinsworldcom
Yes, I saw the coversation and I do agree that UP/DOWN is more intuitive and ideal.
But it needs to hack the code in Scintilla to do that.
I would say, let's add 2 shortcuts directly, so it's usable immediately - if users do need change or there's some conflict they can change them via the ShorcutMapper.

donho · 2022-08-22T21:44:26Z

PowerEditor/src/WinControls/shortcut/shortcut.cpp

@@ -1235,7 +1235,7 @@ CommandShortcut::CommandShortcut(const Shortcut& sc, long id) :	Shortcut(sc), _i
 		_category = TEXT("File");
 	else if ( _id < IDM_SEARCH)
 		_category = TEXT("Edit");
-	else if (_id >= IDM_EDIT_AUTOCOMPLETE and _id <= IDM_EDIT_AUTOCOMPLETE_PATH)
+	else if (_id >= IDM_EDIT_AUTOCOMPLETE and _id <= IDM_EDIT_AUTOCOMPLETE_PATH or _id == IDM_EDIT_FUNCCALLTIP_NEXT)


I know you just follow the style, but it's the old style which should be changed. Let's do:
else if ((_id >= IDM_EDIT_AUTOCOMPLETE) && (_id <= IDM_EDIT_AUTOCOMPLETE_PATH) || (_id == IDM_EDIT_FUNCCALLTIP_NEXT))

vinsworldcom · 2022-08-22T23:14:04Z

Please add new entries into "english.xml" and consider the given suggestions.

@donho - just pushed a new commit with your requested changes. I did compile and test myself and the previous / next seemed to work as expected once I manually mapped shortcuts in the "Shortcut Mapper...".

Please pay attention to the else if() in shortcut.cpp; since we added both previous and next, it required a more "complex" conditional and I want to make sure you like the format I used.

Cheers.

donho · 2022-08-23T16:59:32Z

@vinsworldcom

Please pay attention to the else if() in shortcut.cpp; since we added both previous and next, it required a more "complex" conditional and I want to make sure you like the format I used.

It's OK for this PR at least.

donho

Add ALT-UP & ALT-DOWN as their default shortcuts.

donho · 2022-08-23T16:26:50Z

PowerEditor/src/Parameters.cpp

@@ -149,6 +149,7 @@ static const WinMenuKeyDefinition winKeyDefs[] =
 	{ VK_SPACE,   IDM_EDIT_AUTOCOMPLETE_PATH,                   true,  true,  false, nullptr },
 	{ VK_RETURN,  IDM_EDIT_AUTOCOMPLETE_CURRENTFILE,            true,  false, false, nullptr },
 	{ VK_SPACE,   IDM_EDIT_FUNCCALLTIP,                         true,  false, true,  nullptr },
+	{ VK_NULL,    IDM_EDIT_FUNCCALLTIP_NEXT,                    false, false, false, nullptr },


@vinsworldcom
Yes, I saw the coversation and I do agree that UP/DOWN is more intuitive and ideal.
But it needs to hack the code in Scintilla to do that.
I would say, let's add 2 shortcuts directly, so it's usable immediately - if users do need change or there's some conflict they can change them via the ShorcutMapper.

vinsworldcom · 2022-08-23T17:44:18Z

@donho

Add ALT-UP & ALT-DOWN as their default shortcuts.

Done! Tested on local portable 64-bit install - worked OK. AppVeyor is running now ...

Yes, I saw the coversation and I do agree that UP/DOWN is more intuitive and ideal. But it needs to hack the code in Scintilla to do that.

That was what I thought - I see the Scintilla code for AutoComplete essentially instantiates a combobox which gets the Up / Down arrows handled. I don't see where Scintilla CallTip source instantiates a Windows control - assuming it draws some kind of window, but again, don't see the callback handler so I thought some hacking would be needed for Scintilla to capture Up / Down arrows.

Alas, happy to see this feature make it into N++!

Cheers.

alankilborn · 2022-08-23T18:14:29Z

Yes, I saw the coversation and I do agree that UP/DOWN is more intuitive and ideal. But it needs to hack the code in Scintilla to do that.

That was what I thought - I see the Scintilla code for AutoComplete essentially instantiates a combobox which gets the Up / Down arrows handled. I don't see where Scintilla CallTip source instantiates a Windows control - assuming it draws some kind of window, but again, don't see the callback handler so I thought some hacking would be needed for Scintilla to capture Up / Down arrows.

Perhaps a Scintilla feature request, if @vinsworldcom feels like making one? I don't care enough about this feature to bother.

VinsWorldcom added 2 commits August 15, 2022 14:45

Function Completion menuitem to cycle through choices; Fix 11950

8b0dca0

add to shortcut mapper

e333cd8

dfs- reviewed Aug 16, 2022

View reviewed changes

chcg added the feature Feature requests and Feature commits label Aug 17, 2022

donho self-assigned this Aug 22, 2022

donho requested changes Aug 22, 2022

View reviewed changes

@donho requested changes

dba2a05

vinsworldcom requested a review from donho August 23, 2022 15:46

donho requested changes Aug 23, 2022

View reviewed changes

@donho requested changes - add shortcuts Alt+UP, Alt+DOWN

7a0db3c

vinsworldcom requested a review from donho August 23, 2022 17:40

donho closed this in 8519003 Aug 24, 2022

donho approved these changes Aug 24, 2022

View reviewed changes

vinsworldcom deleted the 11950 branch August 24, 2022 14:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Function Completion menu item to cycle through choices #12017

Function Completion menu item to cycle through choices #12017

vinsworldcom commented Aug 15, 2022 •

edited

Loading

dfs- Aug 16, 2022

vinsworldcom Aug 16, 2022

donho commented Aug 22, 2022

vinsworldcom commented Aug 22, 2022

alankilborn commented Aug 22, 2022

vinsworldcom commented Aug 22, 2022 •

edited

Loading

alankilborn commented Aug 22, 2022

vinsworldcom commented Aug 22, 2022 •

edited

Loading

donho left a comment

donho Aug 22, 2022

donho Aug 22, 2022

donho Aug 22, 2022

vinsworldcom Aug 22, 2022

donho Aug 23, 2022

donho Aug 22, 2022

vinsworldcom commented Aug 22, 2022

donho commented Aug 23, 2022

donho left a comment

donho Aug 23, 2022

vinsworldcom commented Aug 23, 2022 •

edited

Loading

alankilborn commented Aug 23, 2022

	#define IDCMD 50000
	//#define IDM_EDIT_AUTOCOMPLETE (IDCMD+0)
	//#define IDM_EDIT_AUTOCOMPLETE_CURRENTFILE (IDCMD+1)

	#define IDC_PREV_DOC (IDCMD+3)
	#define IDC_NEXT_DOC (IDCMD+4)
	#define IDC_EDIT_TOGGLEMACRORECORDING (IDCMD+5)
	//#define IDC_KEY_HOME (IDCMD+6)
	//#define IDC_KEY_END (IDCMD+7)
	//#define IDC_KEY_SELECT_2_HOME (IDCMD+8)
	//#define IDC_KEY_SELECT_2_END (IDCMD+9)

Function Completion menu item to cycle through choices #12017

Function Completion menu item to cycle through choices #12017

Conversation

vinsworldcom commented Aug 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donho commented Aug 22, 2022

vinsworldcom commented Aug 22, 2022

alankilborn commented Aug 22, 2022

vinsworldcom commented Aug 22, 2022 • edited Loading

alankilborn commented Aug 22, 2022

vinsworldcom commented Aug 22, 2022 • edited Loading

donho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vinsworldcom commented Aug 22, 2022

donho commented Aug 23, 2022

donho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vinsworldcom commented Aug 23, 2022 • edited Loading

alankilborn commented Aug 23, 2022

vinsworldcom commented Aug 15, 2022 •

edited

Loading

vinsworldcom commented Aug 22, 2022 •

edited

Loading

vinsworldcom commented Aug 22, 2022 •

edited

Loading

vinsworldcom commented Aug 23, 2022 •

edited

Loading