From 7cb3ab50581cba45aaf57eeb0ac45bdf3e7b5912 Mon Sep 17 00:00:00 2001 From: Yee Cheng Chin Date: Thu, 27 Mar 2025 23:59:39 -0700 Subject: [PATCH] Fix macOS services no longer able to insert texts in non-Visual modes Interactions with OS services that insert texts were improved in #1552 to make it work in a more integrated fashion instead of a hacky injection of 's' followed by the text. However, it only accounted for services that replaces selected texts in visual mode. In other modes, MacVim would simply ignore the service. This was a regression as previously it would work everywhere (albeit often times in a non-intuitive manner since if used in insert mode the user would see an 's' in the beginning). This also affects Shortcuts that a user may have made that could be invoked from the Services menu or bound to a hotkey. Fix this properly by allowing this to be used in Normal / Insert / Cmdline modes in addition to Visual mode. Even with this fix, there is still a slight difference between the new behavior and the old hacky solution - the old method would leave the user in Insert mode, whereas the new method would stay in Normal mode. I think this is an improvement so no need to fix. Fix #1569 --- src/MacVim/MMBackend.m | 104 +++++++++++++++++++++++---- src/MacVim/MMTextViewHelper.m | 2 +- src/MacVim/MMVimController.h | 2 +- src/MacVim/MMVimController.m | 6 +- src/MacVim/MMWindowController.m | 2 +- src/MacVim/MacVim.h | 2 +- src/MacVim/MacVimTests/MacVimTests.m | 73 ++++++++++++++++--- 7 files changed, 161 insertions(+), 30 deletions(-) diff --git a/src/MacVim/MMBackend.m b/src/MacVim/MMBackend.m index b443d8bec3..824ed659f3 100644 --- a/src/MacVim/MMBackend.m +++ b/src/MacVim/MMBackend.m @@ -1443,10 +1443,11 @@ - (NSString *)selectedText return nil; } -/// Replace the selected text in visual mode with the new suppiled one. -- (oneway void)replaceSelectedText:(in bycopy NSString *)text +/// Insert or replace text with the supplied text. Works in Normal / Visual / +/// Insert / Cmdline modes. +- (oneway void)insertOrReplaceSelectedText:(in bycopy NSString *)text { - if (VIsual_active && (State & MODE_NORMAL)) { + if (State & MODE_NORMAL || State & MODE_INSERT) { // The only real way Vim has in doing this consistently is to use the // register put functionality as there is no generic API for this. // We find an arbitrary register ('0'), back it up, replace it with our @@ -1474,17 +1475,27 @@ - (oneway void)replaceSelectedText:(in bycopy NSString *)text write_reg_contents_ex('0', vimtext, -1, FALSE, yank_type, -1); vim_free(vimtext); - oparg_T oap; - CLEAR_FIELD(oap); - oap.regname = '0'; - - cmdarg_T cap; - CLEAR_FIELD(cap); - cap.oap = &oap; - cap.cmdchar = 'P'; - cap.count1 = 1; + if (State & MODE_NORMAL || State & MODE_INSERT) { + oparg_T oap; + CLEAR_FIELD(oap); + oap.regname = '0'; + + cmdarg_T cap; + CLEAR_FIELD(cap); + cap.oap = &oap; + if (State & MODE_NORMAL) { + // Do 'P' or 'v_P' depending if we are in visual mode. They both do + // the correct behaviors, so no need to check for VIsual_active. + cap.cmdchar = 'P'; + } else { + // Need 'gP' to leave the cursor at the right location. + cap.cmdchar = 'g'; + cap.nchar = 'P'; + } + cap.count1 = 1; - nv_put(&cap); + nv_put(&cap); + } // Clean up the temporary register, and restore the old state. yankreg_T *old_y_current = get_y_current(); @@ -1496,6 +1507,73 @@ - (oneway void)replaceSelectedText:(in bycopy NSString *)text // nv_put does not trigger a redraw command as it's done on a higher // level, so just do a manual one here to make sure it's done. [self redrawScreen]; + } else if (State & MODE_CMDLINE) { + // This is basically doing the following: + // - let cmdline_str = getcmdline() + // - let cmdline_pos = getcmdpos() - 1 + // - setcmdline(cmdline_str[0:cmdline_pos] .. text .. cmdline_str[cmdline_pos:], cmdline_pos + len(text) + 1) + + typval_T cmdline_str; + f_getcmdline(NULL, &cmdline_str); + + typval_T cmdline_pos; + f_getcmdpos(NULL, &cmdline_pos); + + char_u *vimtext = [text vimStringSave]; + for (char_u *c = vimtext; *c != NUL; c++) { + // Perform NL conversion due to Vim's internal usage + if (*c == '\n') + *c = '\r'; + } + + size_t new_size = STRLEN(vimtext); + varnumber_T pos = new_size + 1; + + if (cmdline_str.vval.v_string != NULL && cmdline_pos.vval.v_number != 0) { + // Combine original string with new one + char_u *orig_str = cmdline_str.vval.v_string; + size_t pos_index = cmdline_pos.vval.v_number - 1; + + size_t orig_size = STRLEN(cmdline_str.vval.v_string); + + if (pos_index > orig_size) + pos_index = orig_size; // shouldn't really happen + + char_u *newtext = alloc(orig_size + new_size + 1); + if (pos_index > 0) + memcpy(newtext, orig_str, pos_index); + memcpy(newtext + pos_index, vimtext, new_size); + if (pos_index < orig_size) + memcpy(newtext + pos_index + new_size, orig_str + pos_index, orig_size - pos_index); + newtext[orig_size + new_size] = '\0'; + + vim_free(vimtext); + vimtext = newtext; + + pos += pos_index; + } + + { + typval_T arg_cmdline_str_new; + init_tv(&arg_cmdline_str_new); + arg_cmdline_str_new.v_type = VAR_STRING; + arg_cmdline_str_new.vval.v_string = vimtext; + + typval_T arg_cmdline_pos; + init_tv(&arg_cmdline_pos); + arg_cmdline_pos.v_type = VAR_NUMBER; + arg_cmdline_pos.vval.v_number = pos; + + typval_T args[2] = { arg_cmdline_str_new, arg_cmdline_pos }; + + typval_T ret; + f_setcmdline(args, &ret); + + vim_free(vimtext); + } + + if (cmdline_str.vval.v_string != NULL) + vim_free(cmdline_str.vval.v_string); } } diff --git a/src/MacVim/MMTextViewHelper.m b/src/MacVim/MMTextViewHelper.m index 533a1b40c6..abcf514b8f 100644 --- a/src/MacVim/MMTextViewHelper.m +++ b/src/MacVim/MMTextViewHelper.m @@ -251,7 +251,7 @@ - (void)insertText:(id)string replacementRange:(NSRange)replacementRange // Only known way of this being called is Apple Intelligence Writing // Tools. MMVimController *vc = [self vimController]; - [vc replaceSelectedText:string]; + [vc insertOrReplaceSelectedText:string]; return; } diff --git a/src/MacVim/MMVimController.h b/src/MacVim/MMVimController.h index ec4bc9505c..6a5ea5485e 100644 --- a/src/MacVim/MMVimController.h +++ b/src/MacVim/MMVimController.h @@ -94,7 +94,7 @@ errorString:(NSString **)errstr; - (BOOL)hasSelectedText; - (NSString *)selectedText; -- (void)replaceSelectedText:(NSString *)text; +- (void)insertOrReplaceSelectedText:(NSString *)text; - (void)processInputQueue:(NSArray *)queue; #if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_12_2 - (NSTouchBar *)makeTouchBar; diff --git a/src/MacVim/MMVimController.m b/src/MacVim/MMVimController.m index 05eb56f11a..e48f55f411 100644 --- a/src/MacVim/MMVimController.m +++ b/src/MacVim/MMVimController.m @@ -563,14 +563,14 @@ - (NSString *)selectedText return selectedText; } -- (void)replaceSelectedText:(NSString *)text +- (void)insertOrReplaceSelectedText:(NSString *)text { if (backendProxy) { @try { - [backendProxy replaceSelectedText:text]; + [backendProxy insertOrReplaceSelectedText:text]; } @catch (NSException *ex) { - ASLogDebug(@"replaceSelectedText: failed: pid=%d reason=%@", + ASLogDebug(@"insertOrReplaceSelectedText: failed: pid=%d reason=%@", pid, ex); } } diff --git a/src/MacVim/MMWindowController.m b/src/MacVim/MMWindowController.m index d8faf9e75f..fc977f5ab1 100644 --- a/src/MacVim/MMWindowController.m +++ b/src/MacVim/MMWindowController.m @@ -1679,7 +1679,7 @@ - (BOOL)readSelectionFromPasteboard:(NSPasteboard *)pboard NSArray *types = [pboard types]; if ([types containsObject:NSPasteboardTypeString]) { NSString *input = [pboard stringForType:NSPasteboardTypeString]; - [vimController replaceSelectedText:input]; + [vimController insertOrReplaceSelectedText:input]; return YES; } diff --git a/src/MacVim/MacVim.h b/src/MacVim/MacVim.h index 46dbd2b0ea..6291531677 100644 --- a/src/MacVim/MacVim.h +++ b/src/MacVim/MacVim.h @@ -194,7 +194,7 @@ typedef NSString* NSAttributedStringKey; errorString:(out bycopy NSString **)errstr; - (BOOL)hasSelectedText; - (NSString *)selectedText; -- (oneway void)replaceSelectedText:(in bycopy NSString *)text; +- (oneway void)insertOrReplaceSelectedText:(in bycopy NSString *)text; - (BOOL)mouseScreenposIsSelection:(int)row column:(int)column selRow:(byref int *)startRow selCol:(byref int *)startCol; - (oneway void)acknowledgeConnection; @end diff --git a/src/MacVim/MacVimTests/MacVimTests.m b/src/MacVim/MacVimTests/MacVimTests.m index c7c371e527..213c649e9f 100644 --- a/src/MacVim/MacVimTests/MacVimTests.m +++ b/src/MacVim/MacVimTests/MacVimTests.m @@ -1470,6 +1470,10 @@ - (void)testIPCSelectedText { NSString *regcontents = [vc evaluateVimExpression:@"getreg()"]; XCTAssertEqualObjects(regcontents, @"abcd\n"); + // Visual mode + + NSString *changedtick1 = [vc evaluateVimExpression:@"b:changedtick"]; + // Get selected texts in visual mode XCTAssertFalse([vc hasSelectedText]); XCTAssertNil([vc selectedText]); @@ -1491,30 +1495,79 @@ - (void)testIPCSelectedText { XCTAssertEqualObjects([vc selectedText], @"bc\nfg"); // Set selected texts in visual block mode - NSString *changedtick = [vc evaluateVimExpression:@"b:changedtick"]; - [vc replaceSelectedText:@"xyz\n1234"]; - NSString *changedtick2 = [vc evaluateVimExpression:@"b:changedtick"]; + [vc insertOrReplaceSelectedText:@"xyz\n1234"]; XCTAssertEqualObjects([vc evaluateVimExpression:@"getline(1)"], @"axyz d"); XCTAssertEqualObjects([vc evaluateVimExpression:@"getline(2)"], @"e1234h"); XCTAssertEqualObjects([vc evaluateVimExpression:@"getline(3)"], @"ijkl"); - XCTAssertNotEqualObjects(changedtick, changedtick2); - - // Make sure replacing texts when nothing is selected won't set anything - [vc replaceSelectedText:@"foobar"]; - NSString *changedtick3 = [vc evaluateVimExpression:@"b:changedtick"]; - XCTAssertEqualObjects(changedtick2, changedtick3); // Select in visual block again but send a different number of lines, make sure we intentionaly won't treat it as block text [self sendStringToVim:@"ggjjvll" withMods:0]; [self sendKeyToVim:@"v" withMods:NSEventModifierFlagControl]; [self waitForEventHandlingAndVimProcess]; - [vc replaceSelectedText:@"xyz\n1234\n"]; // ending in newline means it gets interpreted as line-wise + [vc insertOrReplaceSelectedText:@"xyz\n1234\n"]; // ending in newline means it gets interpreted as line-wise XCTAssertEqualObjects([vc evaluateVimExpression:@"getline(1)"], @"axyz d"); XCTAssertEqualObjects([vc evaluateVimExpression:@"getline(2)"], @"e1234h"); XCTAssertEqualObjects([vc evaluateVimExpression:@"getline(3)"], @"xyz"); XCTAssertEqualObjects([vc evaluateVimExpression:@"getline(4)"], @"1234"); XCTAssertEqualObjects([vc evaluateVimExpression:@"getline(5)"], @"l"); + // Normal mode + + // When nothing is selected this will simply insert the text and not replace anything + [self sendStringToVim:@"ggll" withMods:0]; + [self waitForEventHandlingAndVimProcess]; + [vc insertOrReplaceSelectedText:@"_normtext_"]; + XCTAssertEqualObjects([vc evaluateVimExpression:@"getline(1)"], @"ax_normtext_yz d"); + + // Insert mode + + [self sendStringToVim:@"ggjja" withMods:0]; + [self waitForEventHandlingAndVimProcess]; + // Should insert the text at the cursor + [vc insertOrReplaceSelectedText:@"_inserttext_"]; + XCTAssertEqualObjects([vc evaluateVimExpression:@"getline(3)"], @"x_inserttext_yz"); + // Should leave the cursor past the inserted text + [self sendStringToVim:@"additional_text" withMods:0]; + [self waitForEventHandlingAndVimProcess]; + XCTAssertEqualObjects([vc evaluateVimExpression:@"getline(3)"], @"x_inserttext_additional_textyz"); + [self sendKeyToVim:@"[" withMods:NSEventModifierFlagControl]; // escape insert mode + [self waitForEventHandlingAndVimProcess]; + + // Cmdline mode + + NSString *changedtick2 = [vc evaluateVimExpression:@"b:changedtick"]; + XCTAssertNotEqualObjects(changedtick1, changedtick2); + + [self sendStringToVim:@":cnoremap z \n" withMods:0]; + [self sendStringToVim:@":123" withMods:0]; + [self waitForEventHandlingAndVimProcess]; + [vc insertOrReplaceSelectedText:@"a\nb\n"]; + XCTAssertEqualObjects([vc evaluateVimExpression:@"getcmdline()"], @"123a\rb\r"); // Vim does internal \n to \r conversion + XCTAssertEqualObjects([vc evaluateVimExpression:@"getcmdpos()"], @"8"); + [self sendKeyToVim:@"[" withMods:NSEventModifierFlagControl]; // escape cmdline + [self waitForEventHandlingAndVimProcess]; + + [self sendStringToVim:@":123zzz" withMods:0]; + [self waitForEventHandlingAndVimProcess]; + [vc insertOrReplaceSelectedText:@"foobar"]; + XCTAssertEqualObjects([vc evaluateVimExpression:@"getcmdline()"], @"foobar123"); + XCTAssertEqualObjects([vc evaluateVimExpression:@"getcmdpos()"], @"7"); + [self sendKeyToVim:@"[" withMods:NSEventModifierFlagControl]; // escape cmdline + + [self waitForEventHandlingAndVimProcess]; + [self sendStringToVim:@":123z" withMods:0]; + [self waitForEventHandlingAndVimProcess]; + [vc insertOrReplaceSelectedText:@"foobar"]; + XCTAssertEqualObjects([vc evaluateVimExpression:@"getcmdline()"], @"12foobar3"); + XCTAssertEqualObjects([vc evaluateVimExpression:@"getcmdpos()"], @"9"); + [self sendKeyToVim:@"[" withMods:NSEventModifierFlagControl]; // escape cmdline + [self waitForEventHandlingAndVimProcess]; + + // Make sure that the actual buffer wasn't changed at all during these insertions as they all + // went to the cmdline. + NSString *changedtick3 = [vc evaluateVimExpression:@"b:changedtick"]; + XCTAssertEqualObjects(changedtick2, changedtick3); + // Make sure registers didn't get stomped (internally the implementation uses register and manually restores it) regcontents = [[app keyVimController] evaluateVimExpression:@"getreg()"]; XCTAssertEqualObjects(regcontents, @"abcd\n");