From 134b06b65e5f66842cbf5543d0ad1edf45053f24 Mon Sep 17 00:00:00 2001
From: Ben Jackson <puremourning@gmail.com>
Date: Sun, 3 Jan 2016 18:55:27 +0000
Subject: [PATCH] Fix GoTo lists arriving at the wrong column.

Vim's QuickFix lists require 1-based columns, which is what is returned
from ycmd's commands.

 As noted in the comments, the Vim documentation for setqflist is
somewhat vague about this "byte offset", but it is confirmed to mean
"1-based column number" both in testing and in :help getqflist.
---
 python/ycm/client/command_request.py          | 10 +-
 .../ycm/client/tests/command_request_test.py  | 91 +++++++++++++++++++
 python/ycm/vimsupport.py                      |  6 +-
 3 files changed, 104 insertions(+), 3 deletions(-)
 create mode 100644 python/ycm/client/tests/command_request_test.py

diff --git a/python/ycm/client/command_request.py b/python/ycm/client/command_request.py
index 3a4b049a..2c298972 100644
--- a/python/ycm/client/command_request.py
+++ b/python/ycm/client/command_request.py
@@ -135,5 +135,13 @@ def _BuildQfListItem( goto_data_item ):
   if 'line_num' in goto_data_item:
     qf_item[ 'lnum' ] = goto_data_item[ 'line_num' ]
   if 'column_num' in goto_data_item:
-    qf_item[ 'col' ] = goto_data_item[ 'column_num' ] - 1
+    # ycmd returns columns 1-based, and QuickFix lists require "byte offsets".
+    # See :help getqflist and equivalent comment in
+    # vimsupport.ConvertDiagnosticsToQfList.
+    #
+    # When the Vim help says "byte index", it really means "1-based column
+    # number" (which is somewhat confusing). :help getqflist states "first
+    # column is 1".
+    qf_item[ 'col' ] = goto_data_item[ 'column_num' ]
+
   return qf_item
diff --git a/python/ycm/client/tests/command_request_test.py b/python/ycm/client/tests/command_request_test.py
new file mode 100644
index 00000000..b2da8e5d
--- /dev/null
+++ b/python/ycm/client/tests/command_request_test.py
@@ -0,0 +1,91 @@
+#
+# Copyright (C) 2016 YouCompleteMe Contributors
+#
+# This file is part of YouCompleteMe.
+#
+# YouCompleteMe is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# YouCompleteMe is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with YouCompleteMe.  If not, see <http://www.gnu.org/licenses/>.
+
+from ycm.test_utils import MockVimModule
+MockVimModule()
+
+from mock import patch, call
+from ycm.client.command_request import CommandRequest
+
+
+class GoToResponse_QuickFix_test:
+  """This class tests the generation of QuickFix lists for GoTo responses which
+  return multiple locations, such as the Python completer and JavaScript
+  completer. It mostly proves that we use 1-based indexing for the column
+  number."""
+
+  def setUp( self ):
+    self._request = CommandRequest( [ 'GoToTest' ] )
+
+
+  def tearDown( self ):
+    self._request = None
+
+
+  def GoTo_EmptyList_test( self ):
+    self._CheckGoToList( [], [] )
+
+
+  def GoTo_SingleItem_List_test( self ):
+    self._CheckGoToList( [ {
+      'filepath':     'dummy_file',
+      'line_num':     10,
+      'column_num':   1,
+      'description': 'this is some text',
+    } ], [ {
+      'filename':    'dummy_file',
+      'text':        'this is some text',
+      'lnum':        10,
+      'col':         1
+    } ] )
+
+
+  def GoTo_MultiItem_List_test( self ):
+    self._CheckGoToList( [ {
+      'filepath':     'dummy_file',
+      'line_num':     10,
+      'column_num':   1,
+      'description': 'this is some other text',
+    }, {
+      'filepath':     'dummy_file2',
+      'line_num':     1,
+      'column_num':   21,
+      'description': 'this is some text',
+    } ], [ {
+      'filename':    'dummy_file',
+      'text':        'this is some other text',
+      'lnum':        10,
+      'col':         1
+    }, {
+      'filename':    'dummy_file2',
+      'text':        'this is some text',
+      'lnum':        1,
+      'col':         21
+    } ] )
+
+
+  @patch( 'vim.eval' )
+  def _CheckGoToList( self, completer_response, expected_qf_list, vim_eval ):
+    self._request._response = completer_response
+
+    self._request.RunPostCommandActionsIfNeeded()
+
+    vim_eval.assert_has_calls( [
+      call( 'setqflist( {0} )'.format( repr( expected_qf_list ) ) ),
+      call( 'youcompleteme#OpenGoToList()' ),
+    ] )
diff --git a/python/ycm/vimsupport.py b/python/ycm/vimsupport.py
index be8ee338..59dee49c 100644
--- a/python/ycm/vimsupport.py
+++ b/python/ycm/vimsupport.py
@@ -240,10 +240,12 @@ def SetLocationList( diagnostics ):
 
 def ConvertDiagnosticsToQfList( diagnostics ):
   def ConvertDiagnosticToQfFormat( diagnostic ):
-    # see :h getqflist for a description of the dictionary fields
+    # See :h getqflist for a description of the dictionary fields.
     # Note that, as usual, Vim is completely inconsistent about whether
     # line/column numbers are 1 or 0 based in its various APIs. Here, it wants
-    # them to be 1-based.
+    # them to be 1-based. The documentation states quite clearly that it
+    # expects a byte offset, by which it means "1-based column number" as
+    # described in :h getqflist ("the first column is 1").
     location = diagnostic[ 'location' ]
     line_num = location[ 'line_num' ]