tomtom / quickfixsigns_vim

Mark quickfix & location list items with signs
http://www.vim.org/scripts/script.php?script_id=2584
GNU General Public License v3.0
131 stars 13 forks source link

Invalid lnum when using Fugitive Glog command #16

Closed mutewinter closed 12 years ago

mutewinter commented 12 years ago

When using Fugitive in conjunction with quickfixsigns I'm seeing the following error print out for every line in the file when running :Glog

Quickfixsigns PlaceSign: Invalid lnum: {'lnum': 0, 'bufnr': 2, 'col': 0, 'valid': 1, 'vcol': 0, 'nr': -1, 'type': '', 'pattern': '', 'text': 'Commit message here.'}

@rmunn tracked down the error and reported it on my dot_vim repository. Here's what he said could fix the problem:

Possible fixes might involve patching quickfixsigns not to complain when lnum = 0 -- the following patch to quickfixsigns seems to work:

diff --git a/plugin/quickfixsigns.vim b/plugin/quickfixsigns.vim
index 9215b34..79f384e 100755
--- a/plugin/quickfixsigns.vim
+++ b/plugin/quickfixsigns.vim
@@ -614,9 +614,9 @@ function! s:PlaceSign(class, sign, list) "{{{3
                     endif
                 endif
             else
-                echohl WarningMsg
-                echom "Quickfixsigns PlaceSign: Invalid lnum:" string(item)
-                echohl NONE
+                "echohl WarningMsg
+                "echom "Quickfixsigns PlaceSign: Invalid lnum:" string(item)
+                "echohl NONE
             endif
         endfor
     finally
rmunn commented 12 years ago

Note that my patch merely silences the error message, but doesn't do anything about the cause. However, I think in the case of this particular interaction the root cause isn't a bug, just different expectations about what the quickfix list will contain. The quickfixsigns.vim code expects the quickfix list to be populated by things like compilation errors, which come with line numbers -- and if those line numbers are missing, it's a bug. But when the quickfix list is being populated by fugitive via the :Glog command (which runs git log), it's populating the quickfix list from the Git log, which means that the absence of line numbers is expected.

So depending on what is populating the quickfix list, the absence of line numbers may or may not be a bug, which means that my quick-and-dirty hack of a patch, which just silences the error message, will not be the right solution in every case. But it's certainly enough to remove the pain of using Fugitive and quickfixsigns.vim together, which is all I cared about earlier today when I was trying to look at some git logs. So take this patch with a grain of salt. :-)

tomtom commented 12 years ago

But why does fugitive populate the quickfixlist in the first place?

mutewinter commented 12 years ago

Thanks for the quick fix @tomtom!