From 565a8a17adfe2edcd020c8431e0ca32b9f6f4d59 Mon Sep 17 00:00:00 2001 From: Yee Cheng Chin Date: Thu, 26 Oct 2023 23:09:45 -0700 Subject: [PATCH] Fix Help menu searching Vim doc not working with special chars Previously Help menu's Vim doc search did not work with special key documentation like `` because when sending the input to Vim, Vim interpreted `:h ` as physically pressing down arrow instead of typing the literal text in. Fix this by a somewhat manual process of splitting the strings up and using `:execute` and string concatenation. It's not pretty but works. Otherwise we would need to define another IPC call to be able to pass commands to Vim without escaping commands. Also fix up XCText utility so we don't use a predicate to wait for Vim open/close which was previously kind of slow. The new method of just using swizzling is much faster. --- src/MacVim/MMAppController.m | 27 +++++- src/MacVim/MacVim.xctestplan | 1 + src/MacVim/MacVimTests/MacVimTests.m | 128 +++++++++++++++++++++++---- 3 files changed, 135 insertions(+), 21 deletions(-) diff --git a/src/MacVim/MMAppController.m b/src/MacVim/MMAppController.m index 9afacf7a72..7473b9d25b 100644 --- a/src/MacVim/MMAppController.m +++ b/src/MacVim/MMAppController.m @@ -1676,6 +1676,8 @@ - (NSArray *)serverList return item; } +/// Invoked when user typed on the help menu search bar. Will parse doc tags +/// and search among them for the search string and return the match items. - (void)searchForItemsWithSearchString:(NSString *)searchString resultLimit:(NSInteger)resultLimit matchedItemHandler:(void (^)(NSArray *items))handleMatchedItems @@ -1748,6 +1750,8 @@ - (void)searchForItemsWithSearchString:(NSString *)searchString handleMatchedItems(ret); } +/// Invoked when user clicked on a Help menu item for a documentation tag +/// previously returned by searchForItemsWithSearchString. - (void)performActionForItem:(id)item { // When opening a help page, either open a new Vim instance, or reuse the @@ -1758,8 +1762,27 @@ - (void)performActionForItem:(id)item @":help %@", item[1]]]; return; } - [vimController addVimInput:[NSString stringWithFormat: - @":help %@", item[1]]]; + + // Vim is already open. We want to send it a message to open help. However, + // we're using `addVimInput`, which always treats input like "" as a key + // while we want to type it literally. The only way to do so is to manually + // split it up and concatenate the results together and pass it to :execute. + NSString *helpStr = item[1]; + + NSMutableString *cmd = [NSMutableString stringWithCapacity:40 + helpStr.length]; + [cmd setString:@":exe 'help "]; + + NSArray *splitComponents = [helpStr componentsSeparatedByString:@"<"]; + for (NSUInteger i = 0; i < splitComponents.count; i++) { + if (i != 0) { + [cmd appendString:@"<'..'"]; + } + NSString *component = splitComponents[i]; + component = [component stringByReplacingOccurrencesOfString:@"'" withString:@"''"]; + [cmd appendString:component]; + } + [cmd appendString:@"'"]; + [vimController addVimInput:cmd]; } // End NSUserInterfaceItemSearching diff --git a/src/MacVim/MacVim.xctestplan b/src/MacVim/MacVim.xctestplan index 31c2a80b81..0ee012ce1f 100644 --- a/src/MacVim/MacVim.xctestplan +++ b/src/MacVim/MacVim.xctestplan @@ -17,6 +17,7 @@ "argument" : "-MMUntitledWindow 0" } ], + "testExecutionOrdering" : "random", "testTimeoutsEnabled" : true }, "testTargets" : [ diff --git a/src/MacVim/MacVimTests/MacVimTests.m b/src/MacVim/MacVimTests/MacVimTests.m index 05a191349c..823839652c 100644 --- a/src/MacVim/MacVimTests/MacVimTests.m +++ b/src/MacVim/MacVimTests/MacVimTests.m @@ -44,17 +44,44 @@ @interface MacVimTests : XCTestCase @implementation MacVimTests -/// Wait for a Vim controller to be added/removed. By the time this is fulfilled -/// the Vim window should be ready and visible. -- (void)waitForVimController:(int)delta { - NSArray *vimControllers = [MMAppController.sharedInstance vimControllers]; - const int desiredCount = (int)vimControllers.count + delta; - [self waitForExpectations:@[[[XCTNSPredicateExpectation alloc] - initWithPredicate:[NSPredicate predicateWithBlock:^(id vimControllers, NSDictionary *bindings) { - return (BOOL)((int)[(NSArray*)vimControllers count] == desiredCount); - }] - object:vimControllers]] - timeout:5]; +/// Wait for Vim window to open and is ready to go +- (void)waitForVimOpen { + XCTestExpectation *expectation = [self expectationWithDescription:@"VimOpen"]; + + SEL sel = @selector(windowControllerWillOpen:); + Method method = class_getInstanceMethod([MMAppController class], sel); + + IMP origIMP = method_getImplementation(method); + IMP newIMP = imp_implementationWithBlock(^(id self, MMWindowController *w) { + typedef void (*fn)(id,SEL,MMWindowController*); + ((fn)origIMP)(self, sel, w); + [expectation fulfill]; + }); + + method_setImplementation(method, newIMP); + [self waitForExpectations:@[expectation] timeout:10]; + method_setImplementation(method, origIMP); + + [self waitForEventHandlingAndVimProcess]; +} + +/// Wait for a Vim window to be closed +- (void)waitForVimClose { + XCTestExpectation *expectation = [self expectationWithDescription:@"VimClose"]; + + SEL sel = @selector(removeVimController:); + Method method = class_getInstanceMethod([MMAppController class], sel); + + IMP origIMP = method_getImplementation(method); + IMP newIMP = imp_implementationWithBlock(^(id self, id controller) { + typedef void (*fn)(id,SEL,id); + ((fn)origIMP)(self, sel, controller); + [expectation fulfill]; + }); + + method_setImplementation(method, newIMP); + [self waitForExpectations:@[expectation] timeout:10]; + method_setImplementation(method, origIMP); } /// Wait for event handling to be finished at the main loop. @@ -232,7 +259,7 @@ - (void)testVimTutor { // Adding a new window is necessary for the vimtutor menu to show up as it's // not part of the global menu [app openNewWindow:NewWindowClean activate:YES]; - [self waitForVimController:1]; + [self waitForVimOpen]; // Find the vimtutor menu and run it. NSMenu *mainMenu = [NSApp mainMenu]; @@ -248,16 +275,79 @@ - (void)testVimTutor { [[[app keyVimController] windowController] vimMenuItemAction:vimTutorMenu]; // Make sure the menu item actually opened a new window and point to a tutor buffer - [self waitForVimController:1]; + // Note that `vimtutor` opens Vim twice. Once to copy the file. Another time to + // actually open the copied file. + [self waitForVimOpen]; + [self waitForVimOpen]; NSString *bufname = [[app keyVimController] evaluateVimExpression:@"bufname()"]; XCTAssertTrue([bufname containsString:@"tutor"]); // Clean up [[app keyVimController] sendMessage:VimShouldCloseMsgID data:nil]; - [self waitForVimController:-1]; + [self waitForVimClose]; [[app keyVimController] sendMessage:VimShouldCloseMsgID data:nil]; - [self waitForVimController:-1]; + [self waitForVimClose]; + + XCTAssertEqual(0, [app vimControllers].count); +} + +/// Test that opening Vim documentation from Help menu works as expected even +/// with odd characters. +- (void)testHelpMenuDocumentationTag { + MMAppController *app = MMAppController.sharedInstance; + XCTAssertEqual(0, app.vimControllers.count); + + [NSApp activateIgnoringOtherApps:YES]; + + // Test help menu when no window is shown + [app performActionForItem:@[@"", @"m'"]]; + [self waitForVimOpen]; + MMVimController *vim = [app keyVimController]; + + XCTAssertEqualObjects(@"help", [vim evaluateVimExpression:@"&buftype"]); + NSString *curLine = [vim evaluateVimExpression:@"getline('.')"]; + XCTAssertTrue([curLine containsString:@"*m'*"]); + [vim sendMessage:VimShouldCloseMsgID data:nil]; + vim = nil; + [self waitForVimClose]; + + // Test help menu when there's already a Vim window + [app openNewWindow:NewWindowClean activate:YES]; + [self waitForVimOpen]; + vim = [app keyVimController]; + +#define ASSERT_HELP_PATTERN(pattern) \ +do { \ + [app performActionForItem:@[@"foobar.txt", @pattern]]; \ + [self waitForVimProcess]; \ + XCTAssertEqualObjects(@"help", [vim evaluateVimExpression:@"&buftype"]); \ + curLine = [vim evaluateVimExpression:@"getline('.')"]; \ + XCTAssertTrue([curLine containsString:@("*" pattern "*")]); \ +} while(0) + + ASSERT_HELP_PATTERN("macvim-touchbar"); + ASSERT_HELP_PATTERN("++enc"); + ASSERT_HELP_PATTERN("v_CTRL-\\_CTRL-G"); + ASSERT_HELP_PATTERN("/\\%"); + ASSERT_HELP_PATTERN("c__"); + + // single-quote characters should be escaped properly when passed to help + ASSERT_HELP_PATTERN("'display'"); + ASSERT_HELP_PATTERN("m'"); + + // Test both single-quote and '<' + ASSERT_HELP_PATTERN("/\\%<'m"); + ASSERT_HELP_PATTERN("'<"); + +#undef ASSERT_HELP_PATTERN + + // Clean up + [vim sendMessage:VimShouldCloseMsgID data:nil]; + [self waitForVimClose]; } /// Test that cmdline row calculation (used by MMCmdLineAlignBottom) is correct. @@ -268,7 +358,7 @@ - (void) testCmdlineRowCalculation { MMAppController *app = MMAppController.sharedInstance; [app openNewWindow:NewWindowClean activate:YES]; - [self waitForVimController:1]; + [self waitForVimOpen]; MMTextView *textView = [[[[app keyVimController] windowController] vimView] textView]; const int numLines = [textView maxRows]; @@ -276,11 +366,11 @@ - (void) testCmdlineRowCalculation { // Define convenience macro (don't use functions to preserve line numbers in callstack) #define ASSERT_NUM_CMDLINES(expected) \ -{ \ +do { \ const int cmdlineRow = [[[app keyVimController] objectForVimStateKey:@"cmdline_row"] intValue]; \ const int numBottomLines = numLines - cmdlineRow; \ XCTAssertEqual(expected, numBottomLines); \ -} +} while(0) // Default value [self waitForEventHandlingAndVimProcess]; @@ -328,7 +418,7 @@ - (void) testCmdlineRowCalculation { // Clean up [[app keyVimController] sendMessage:VimShouldCloseMsgID data:nil]; - [self waitForVimController:-1]; + [self waitForVimClose]; } @end