Do not add log output to quickfix list

[mrnugget]

I ran into a problem where the output of log.Println was added to the quickfix window when some tests failed. Here is some example code the reproduce the issue:

// testing_example.go
package main

import (
    "fmt"
    "log"
)

func SayName() string {
    log.Println("log line")
    fmt.Println("fmt line!")
    return "wrong"
}

// testing_example_test.go
package main

import "testing"

func TestSayname(t *testing.T) {
    result := SayName()

    if result != "foobar" {
        t.Error("foobar")
    }
}

When the tests were run with :GoTest or :GoTest! the log.Println line was included in the quickfix list, which looked like this:

My guess was that the regex that's responsible for parsing the output was not meant to parse the log line, but only did so accidently.

So I changed the regex to only parse lines for filenames and linenumbers that begin with at least one whitespace (which log lines do not).

(See the commit message for more details)

What do you think? :)

[nowk]

Could this be something you enable or flag in? Log output can be very handy at times, as well as extremely annoying, particularly with the auto jump feature. But I feel this would be ideal as something one can opt in/out of...?

[mrnugget]

I'm sure it could be enabled. In go#Tool#ShowErrors there is this part:

" [...]
        elseif !empty(errors)
            " Preserve indented lines.
            " This comes up especially with multi-line test output.
            if match(line, '^\s') >= 0
                call add(errors, {"text": line})
            endif
        endif
" [...]

This could be modified to always add every line of text to the quickfix window. Which is exactly what happens if you use :make to compile a C program for example:

But these lines are not added with a linenumber and a filename. Just the text, which would be the correct thing to do. The way log output is currently parsed incorrectly is definitely a bug. The regex is meant to only parse filename.go:linenumber: text.

I'd say let's fix it first and then look at how to enable all output in the quickfix window.

[mrnugget]

I think that https://github.com/fatih/vim-go/pull/547/files is related to this issue, albeit it's triggered by building/running a Go program and not testing. But I think merging this change here would fix the problems in PR 547 too, with a much simpler solution.

[mrnugget]

Okay, I just realized that this PR would break the behaviour of :GoVet/:GoRun/:GoBuild since they output their errors without whitespace at the beginning of the line.

I think maybe the best approach would be to add a separate ShowTestErrors function that only parses test output (which is quite different than other output) and takes arguments to show the full output.

[mrnugget]

So, just to clarify: do not merge this, since it breaks the other commands. I'll keep this open for now, so maybe we can discuss this problem. But, if it's the better approach, we can move the discussion to #547.

[fatih]

Hi @mrnugget

The issue #547 issue is quite good. I've tested it with the example file on #287 and it works perfect. I'm closing this so we can focus on #547. Thanks