sds / overcommit

A fully configurable and extendable Git hook manager
MIT License
3.91k stars 280 forks source link

Add option for verbose/debugging output #221

Closed jawshooah closed 9 years ago

jawshooah commented 9 years ago

This would make debugging #220, and probably many future issues, much easier. Could be implemented as a config setting, a CLI option, or both.

sds commented 9 years ago

Can you elaborate on what kind of output you would expect to see with this flag? It would help give a better understanding of the scope of this request.

I could see us adding a debug_log helper which only prints output when an environment variable OVERCOMMIT_DEBUG is set (I'm against using a CLI flag since it would only allow you to use it when invoking via overcommit --run or when installing). We could then call debug_log in execute and other low-level helpers to output the command that was executed along with STDOUT/STDERR/exit status.

jawshooah commented 9 years ago

Sorry to take so long to get back. Yes, printing the executed command along with its output and exit status is what I had in mind. And I imagine that more use cases will crop up as more issues reveal themselves over time.

We could also add a notice to CONTRIBUTING.md about including debug output in bug reports.

blech75 commented 9 years ago

I'd also appreciate some level of verbose output. applicable_files perhaps?

My use case is to see the individual files being passed to each hook to validate that include is targeting the intended files.

(I'm having an issue and it's unclear whether it's the path/glob specified in include, the tool called by overcommit, or overcommit itself. When I figure out what's going on, I'll log an issue or PR.)

jawshooah commented 9 years ago

@blech75 That should be fairly straightforward to diagnose, at least as far as determining which tool is causing the problem. You could shopt -s globstar, then ls <include-patterns> to check the globbing, and <linter-tool> <include-patterns> to check the tool.

blech75 commented 9 years ago

@jawshooah So I spent some time exploring the codebase and debugging my problem. It's clear that accommodating additional debug output is somewhat non-trivial and would require some planning so as not to break any downstream tools.

Re: debugging my issue... Turns out my glob was correct and applicable_files was as expected. It is the underlying tool that changed its output stream from STDERR to STDOUT (semistandard, which in turn means standard), which broke the hook. I'll submit a PR shortly to fix that issue.

It also turns out the output from the command was useful to review, as it changed and broke things in a different way. So, I agree with your suggestion re: outputting debug messages with the exact command along with its output (instead of my request for just the applicable_files). Such output would make diagnosing these types of issues much faster.

BTW, thanks for the tip about shopt -s globstar.

jawshooah commented 9 years ago

Nice catch! Pretty annoying that this change doesn't appear to be documented anywhere outside of feross/standard#113.

And yeah, globstar is a handy little shell option. I just set it in my bash_profile so I don't have to worry about it.

sds commented 9 years ago

Added support for some simple output of exit status and standard output/error streams by specifying the OVERCOMMIT_DEBUG environment variable in 3f660a7.