Add -v flag for verbose log to stdout

[sywhang]

This implements #7. User can specify -v or --verbose flag to make gopatch print whether each file was patched or not to stdout.

The -v short flag was being used for printing version number so I took it away from --version and put it on --verbose because that seems more conventional with CLI tools..

Example output:

~/ > ./gopatch -v -p temp.patch ./tempdir/...
[gopatch] /Users/sungyoonwhang/go/src/gopatch/tempdir/test1.go : Patch successfully applied.
[gopatch] /Users/sungyoonwhang/go/src/gopatch/tempdir/test2.go : Couldn't patch. The file couldn't be parsed.
[gopatch] /Users/sungyoonwhang/go/src/gopatch/tempdir/test3.go : Couldn't patch. Nothing was matched.
[gopatch] /Users/sungyoonwhang/go/src/gopatch/tempdir/test4.go : Patch successfully applied.
could not parse "/Users/sungyoonwhang/go/src/gopatch/tempdir/test2.go": /Users/sungyoonwhang/go/src/gopatch/tempdir/test2.go:2:1: expected 'package', found 'import' (and 1 more errors)

[codecov-commenter]

Codecov Report

Merging #22 (beec6bb) into main (1592705) will decrease coverage by 0.35%.
The diff coverage is 50.00%.

❗ Current head beec6bb differs from pull request most recent head af6949e. Consider uploading reports for the commit af6949e to get more accurate results

@@            Coverage Diff             @@
##             main      #22      +/-   ##
==========================================
- Coverage   86.34%   85.99%   -0.36%     
==========================================
  Files          38       38              
  Lines        1985     1999      +14     
==========================================
+ Hits         1714     1719       +5     
- Misses        217      223       +6     
- Partials       54       57       +3     

Impacted Files	Coverage Δ
main.go	61.53% <50.00%> (-1.14%)	⬇️
internal/engine/changelog.go	91.93% <0.00%> (-3.23%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1592705...af6949e. Read the comment docs.

[abhinav]

Thanks for this change!

The logger we've implemented here roughly matches what the standard library's log package would give us with the following:

log.New(os.Stderr, "[gopatch] ", 0)

We could use that directly.

In a future version, we can use model events through a custom logger type that acts as a boundary between business logic and the output stream. For example, if we did,

type Logger interface {
  ParseError(file string, err error)
  PatchError(file string, err error)
  Patched(file string)
}

Then an implementation could decide the messages and output of each method depending on verbosity and output level. This might become necessary if/when we add trace-level logging on matchers/replacers. Although I'd defer designing the logger interface until we need it.

---

Separately, I think I'd like to avoid the "[gopatch]" prefix completely to reduce output noise; users know what tool they're running, and if part of a script, adding the prefix is easy. So just the following, maybe:

log.New(os.Stder, "", 0) // no prefix, no timestamps

For the log messages themselves:

• we should maybe go for shorter messages
• we can include the errors in the message; this will cause double logging of those errors because we list them at the end, but that's probably okay in verbose mode: you get the error once when it happens, and an error summary at the end

So maybe something like:

path/to/file.go: patched
path/to/other/file.go: skipped
path/to/something.go: failed: [...]

[sywhang]

Thanks for the feedback @abhinav! I've updated the PR to use standard log.Logger instead of implementing a new interface for it since I don't think adding a custom interface is necessary for now as you mentioned.

[abhinav]

Since the comment got hidden because of the suggestion, copying it here:

Separately, do we want verbose output on stdout or stderr? Generally, if the output is meant to be machine processable, it should be stdout so that it's easy to pipe into other commands, but if it's largely for human consumption, it should be stderr. (See also this part of clig.dev.) In this case, I'm open to either.

[sywhang]

Separately, do we want verbose output on stdout or stderr?

I was thinking we should go with stdout because I can imagine the output getting too messy to be human-parsable with large # of files, and I can imagine people piping it for filtering: gopatch -v -p my.patch project/... | grep "skipped" > skipped.log.