Fix bug where clipboard would be polluted when showing Services menu

Previously, whenever the user selects some text in MacVim, and then
bring up the Services menu (either by right-clicking on the selection,
or go to MacVim -> Services), MacVim will copy the selected text to the
system clipboard, which is quite an unexpected behavior. Fix that here.

Part of the issue is that Vim has built-in ways to convert the selected
range to a single copyable text, while managing complexities of dealing
with blockwise/linewise modes. Previous implementation in MacVim was
lazy in that it just invoked those code, but the side effect was that it
would also copy the text to the system clipboard and pollute Vim's star
register as well. This was quite undesirable because the user has not
done anything other than opening a menu and wouldn't have expected the
system clipboard or the Vim registers to have changed.

In this fix, we unfortunately had to do a little bit more copy-pasting
to extract the useful bits so we can copy the text to a register (but
not system clipboard), invoke the code to convert the register to
clipboard-happy text, restore the register, and then move on. A little
annoying but better than having the unintuitive / annoying behaveior
from before, and this way we don't have to do too much intrusive
refactoring of upstream Vim code as well.

Author: ychin

src/MacVim/MMBackend.m

@@ -1357,26 +1357,111 @@ - (NSString *)evaluateExpression:(in bycopy NSString *)expr
     return eval;
 }
 
-- (BOOL)starRegisterToPasteboard:(byref NSPasteboard *)pboard
+/// Extracts the text currently selected in visual mode, and return them in str/len.
+///
+/// @return the motion type (e.g. blockwise), or -1 for failure.
+static int extractSelectedText(char_u **str, long_u *len)
+{
+    // Note: Most of the functionality in Vim that allows for extracting useful
+    // text from a selection are in the register & clipboard utility functions.
+    // Unfortunately, most of those functions would actually send the text to
+    // the system clipboard, which we don't want (since we just want to extract
+    // the text instead of polluting the system clipboard). We don't want to
+    // refactor upstream Vim code too much to avoid merge pains later, so we
+    // duplicate a fair bit of the code from the functions below.
+
+    if (!(VIsual_active && (State & MODE_NORMAL))) {
+        // This only works when we are in visual mode and have stuff to select.
+        return -1;
+    }    
+
+    // Step 1: Find a register to yank the selection to. If we don't do this we
+    // have to duplicate a lot of code in op_yank(). We save it off to a backup
+    // first so we can restore it later to avoid polluting the registers.
+
+    // Just use the yank / '0' register as it makes sense, but it doesn't
+    // really matter.
+    yankreg_T *target_reg = get_y_register(0);
+
+    // Move the contents to the backup without doing memory allocs.
+    yankreg_T backup_reg = *target_reg;
+    target_reg->y_array = NULL;
+    target_reg->y_size = 0;
+
+    // Step 2: Preserve the local states, and then invoke yank.
+    // Note: These were copied from clip_get_selection() in clipboard.c
+    yankreg_T	*old_y_previous, *old_y_current;
+    pos_T	old_cursor;
+    pos_T	old_visual;
+    int		old_visual_mode;
+    colnr_T	old_curswant;
+    int		old_set_curswant;
+    pos_T	old_op_start, old_op_end;
+    oparg_T	oa;
+    cmdarg_T	ca;
+
+    // Avoid triggering autocmds such as TextYankPost.
+    block_autocmds();
+
+    // Yank the selected text into the target register.
+    old_y_previous = get_y_previous();
+    old_y_current = get_y_current();
+    old_cursor = curwin->w_cursor;
+    old_curswant = curwin->w_curswant;
+    old_set_curswant = curwin->w_set_curswant;
+    old_op_start = curbuf->b_op_start;
+    old_op_end = curbuf->b_op_end;
+    old_visual = VIsual;
+    old_visual_mode = VIsual_mode;
+    clear_oparg(&oa);
+    oa.regname = '0'; // Use the '0' (yank) register. We will restore it later to avoid pollution.
+    oa.op_type = OP_YANK;
+    CLEAR_FIELD(ca);
+    ca.oap = &oa;
+    ca.cmdchar = 'y';
+    ca.count1 = 1;
+    ca.retval = CA_NO_ADJ_OP_END;
+    do_pending_operator(&ca, 0, TRUE);
+
+    // Step 3: Convert the yank register to a single piece of useful text. This
+    // will handle all the edge cases of different modes (e.g. blockwise, etc).
+    const int convert_result = clip_convert_selection(str, len, NULL);
+
+    // Step 4: Clean up the yank register, and restore it back.
+    set_y_current(target_reg); // should not be necessary as it's done in do_pending_operator above (since regname was set to 0), but just to be safe and verbose in intention.
+    free_yank_all();
+    *target_reg = backup_reg;
+
+    // Step 5: Restore all the loose states that were modified during yank.
+    // Note: These were copied from clip_get_selection() in clipboard.c
+    set_y_previous(old_y_previous);
+    set_y_current(old_y_current);
+    curwin->w_cursor = old_cursor;
+    changed_cline_bef_curs();   // need to update w_virtcol et al
+    curwin->w_curswant = old_curswant;
+    curwin->w_set_curswant = old_set_curswant;
+    curbuf->b_op_start = old_op_start;
+    curbuf->b_op_end = old_op_end;
+    VIsual = old_visual;
+    VIsual_mode = old_visual_mode;
+
+    unblock_autocmds();
+
+    return convert_result;
+}
+
+/// Extract the currently selected text (in visual mode) and send that to the
+/// provided pasteboard.
+- (BOOL)selectedTextToPasteboard:(byref NSPasteboard *)pboard
 {
-    // TODO: This method should share code with clip_mch_request_selection().
-
-    if (VIsual_active && (State & MODE_NORMAL) && clip_star.available) {
-        // If there is no pasteboard, return YES to indicate that there is text
-        // to copy.
+    if (VIsual_active && (State & MODE_NORMAL)) {
+        // If there is no pasteboard, just return YES to indicate that there is
+        // text to copy.
         if (!pboard)
             return YES;
 
-        // The code below used to be clip_copy_selection() but it is now
-        // static, so do it manually.
-        clip_update_selection(&clip_star);
-        clip_free_selection(&clip_star);
-        clip_get_selection(&clip_star);
-        clip_gen_set_selection(&clip_star);
-
-        // Get the text to put on the pasteboard.
         long_u llen = 0; char_u *str = 0;
-        int type = clip_convert_selection(&str, &llen, &clip_star);
+        int type = extractSelectedText(&str, &llen);
         if (type < 0)
             return NO;
 

src/MacVim/MMWindowController.m

@@ -87,7 +87,7 @@ - (void)resizeWindowToFitContentSize:(NSSize)contentSize
 - (NSSize)constrainContentSizeToScreenSize:(NSSize)contentSize;
 - (NSRect)constrainFrame:(NSRect)frame;
 - (NSTabViewItem *)addNewTabViewItem;
-- (BOOL)askBackendForStarRegister:(NSPasteboard *)pb;
+- (BOOL)askBackendForSelectedText:(NSPasteboard *)pb;
 - (void)updateTablineSeparator;
 - (void)hideTablineSeparator:(BOOL)hide;
 - (void)doFindNext:(BOOL)next;
@@ -1336,21 +1336,25 @@ - (id)validRequestorForSendType:(NSString *)sendType
                      returnType:(NSString *)returnType
 {
     if ([sendType isEqual:NSStringPboardType]
-            && [self askBackendForStarRegister:nil])
+            && [self askBackendForSelectedText:nil])
         return self;
 
     return [super validRequestorForSendType:sendType returnType:returnType];
 }
 
+/// Called by OS when it tries to show a "Services" menu. We ask Vim for the
+/// currently selected text and write that to the provided pasteboard.
 - (BOOL)writeSelectionToPasteboard:(NSPasteboard *)pboard
                              types:(NSArray *)types
 {
     if (![types containsObject:NSStringPboardType])
         return NO;
 
-    return [self askBackendForStarRegister:pboard];
+    return [self askBackendForSelectedText:pboard];
 }
 
+/// Called by the OS when it tries to update the selection. This could happen
+/// if you selected "Convert text to full width" in the Services menu, for example.
 - (BOOL)readSelectionFromPasteboard:(NSPasteboard *)pboard
 {
     // Replace the current selection with the text on the pasteboard.
@@ -1678,18 +1682,25 @@ - (NSTabViewItem *)addNewTabViewItem
     return [vimView addNewTabViewItem];
 }
 
-- (BOOL)askBackendForStarRegister:(NSPasteboard *)pb
-{ 
-    // TODO: Can this be done with evaluateExpression: instead?
+/// Ask Vim to fill in the pasteboard with the currently selected text in visual mode.
+- (BOOL)askBackendForSelectedText:(NSPasteboard *)pb
+{
+    // We use a dedicated API for this instead of just using something like
+    // evaluateExpression because there's a fair bit of logic in Vim that
+    // figures out how to convert from a visual selection to an externally
+    // presentable text in the clipboard code. Part of the complexity has to do
+    // with the three modes (character/blockwise/linewise) that visual mode can
+    // be in. We would like to reuse that code, instead of hacking it via some
+    // expression evaluation results.
     BOOL reply = NO;
     id backendProxy = [vimController backendProxy];
 
     if (backendProxy) {
         @try {
-            reply = [backendProxy starRegisterToPasteboard:pb];
+            reply = [backendProxy selectedTextToPasteboard:pb];
         }
         @catch (NSException *ex) {
-            ASLogDebug(@"starRegisterToPasteboard: failed: pid=%d reason=%@",
+            ASLogDebug(@"selectedTextToPasteboard: failed: pid=%d reason=%@",
                     [vimController pid], ex);
         }
     }

src/MacVim/MacVim.h

@@ -118,7 +118,7 @@
 - (NSString *)evaluateExpression:(in bycopy NSString *)expr;
 - (id)evaluateExpressionCocoa:(in bycopy NSString *)expr
                   errorString:(out bycopy NSString **)errstr;
-- (BOOL)starRegisterToPasteboard:(byref NSPasteboard *)pboard;
+- (BOOL)selectedTextToPasteboard:(byref NSPasteboard *)pboard;
 - (oneway void)acknowledgeConnection;
 @end
 

src/clipboard.c

@@ -2114,7 +2114,19 @@ clip_convert_selection(char_u **str, long_u *len, Clipboard_T *cbd)
     int_u	eolsize;
     yankreg_T	*y_ptr;
 
-    if (cbd == &clip_plus)
+    if (!cbd)
+    {
+	// MacVim extension: This makes this function much more useful as we
+	// can now extract usable texts from any registers for use instead of
+	// being forced to go through the system clipboard. This is useful for
+	// features that expose selected texts (e.g. system services) without
+	// polluting the system clipboard. 
+	int unname_register = get_unname_register();
+	if (unname_register < 0)
+	    return -1;
+	y_ptr = get_y_register(unname_register);
+    }
+    else if (cbd == &clip_plus)
 	y_ptr = get_y_register(PLUS_REGISTER);
     else
 	y_ptr = get_y_register(STAR_REGISTER);