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 `<Down>` because when sending the input to Vim, Vim
interpreted `:h <Down>` 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

@@ -1676,6 +1676,8 @@ fsEventCallback(ConstFSEventStreamRef streamRef,
     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 @@ fsEventCallback(ConstFSEventStreamRef streamRef,
     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 @@ fsEventCallback(ConstFSEventStreamRef streamRef,
                                         @":help %@", item[1]]];
         return;
     }
-    [vimController addVimInput:[NSString stringWithFormat:
-                                @"<C-\\><C-N>:help %@<CR>", 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 "<Up>" 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:@"<C-\\><C-N>:exe 'help "];
+
+    NSArray<NSString*> *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:@"'<CR>"];
+    [vimController addVimInput:cmd];
 }
 // End NSUserInterfaceItemSearching
 

src/MacVim/MacVim.xctestplan

@@ -17,6 +17,7 @@
         "argument" : "-MMUntitledWindow 0"
       }
     ],
+    "testExecutionOrdering" : "random",
     "testTimeoutsEnabled" : true
   },
   "testTargets" : [

src/MacVim/MacVimTests/MacVimTests.m

@@ -44,17 +44,44 @@
 
 @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<NSString *,id> *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 @@
     // 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 @@
     [[[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("/\\%<v");
+
+    // '<' characters need to be concatenated to not be interpreted as keys
+    ASSERT_HELP_PATTERN("c_<Down>");
+    ASSERT_HELP_PATTERN("c_<C-R>_<C-W>");
+
+    // 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 @@
     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 @@
 
     // 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 @@
 
     // Clean up
     [[app keyVimController] sendMessage:VimShouldCloseMsgID data:nil];
-    [self waitForVimController:-1];
+    [self waitForVimClose];
 }
 
 @end