Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quiet mode (pre-commit install --quiet) which outputs less while running #823

Open
graemedavidson opened this issue Sep 5, 2018 · 44 comments
Labels

Comments

@graemedavidson
Copy link

Is it possible to have it only output on Failure, otherwise leaving it clear.

Might be a good additional option (if it does not exist already) to add at the parent level a verbosity setting in which you can get the current Passed/Skipped/Failed output at the highest. a mid level providing a single line of stats --passed=5 --skipped=2 and the above with only failures.

@asottile
Copy link
Member

asottile commented Sep 5, 2018

That's the default behaviour unless you're using --verbose or verbose: true

@graemedavidson
Copy link
Author

graemedavidson commented Sep 5, 2018 via email

@asottile
Copy link
Member

asottile commented Sep 5, 2018

no, it should always show the passed messaging

@graemedavidson
Copy link
Author

graemedavidson commented Sep 5, 2018 via email

@asottile
Copy link
Member

asottile commented Sep 5, 2018

What's the motivation? My initial lean is -0

@asottile
Copy link
Member

Unless you have a good reason I don't want to or plan on implementing this.

Comment back and I'll reopen though :)

@aschokking
Copy link

aschokking commented Feb 21, 2019

I was looking for an option like this (silence the passed hooks) so that there's less noise in my output to scroll through.

Right now this is causing me to not want to have too many separate hooks setup even if they would be useful.

@asottile asottile mentioned this issue Feb 28, 2019
@asottile
Copy link
Member

CC @blueyed

Even if a commandline option were added (which I'm against -- the output is almost a branding at this point), there wouldn't be a way to set it since there aren't ways to pass through arguments from git

@asottile asottile reopened this Feb 28, 2019
@asottile
Copy link
Member

oops, meant to reopen -- should be open now. let's discuss.

@blueyed
Copy link

blueyed commented Feb 28, 2019

I would (like to) configure this in the config file.

@asottile
Copy link
Member

that would change it for all users which I wouldn't want so I don't think that's an appropriate place to change this

@blueyed
Copy link

blueyed commented Feb 28, 2019

So I assume there is no user config then?
What about an environment variable?

@blueyed
Copy link

blueyed commented Feb 28, 2019

A git config option could be used for this.

@asottile
Copy link
Member

I guess another option would be to set it during pre-commit install I forgot about this but we already have a precedent for an option there:

SKIP_ON_MISSING_CONFIG = None

@blueyed
Copy link

blueyed commented Mar 2, 2019

@asottile
Also sounds good.

I came up with the following for me:

--- .git/hooks/pre-commit       2019-10-23 00:52:29.446422989 +0200
+++ .git/hooks/pre-commit-my2   2019-10-23 00:52:25.649698502 +0200
@@ -180,8 +180,29 @@
 def main():
     retv, stdin = _run_legacy()
     try:
+        print('Running pre-commit: ', end='', flush=True)
         _validate_config()
-        return retv | _subprocess_call(_exe() + _opts(stdin))
+        p = subprocess.Popen(_exe() + _opts(stdin) + ('--color=always',),
+                             stdout=subprocess.PIPE)
+        try:
+            p.wait()
+        except KeyboardInterrupt as exc:
+            assert p.returncode == 1
+        except Exception as exc:
+            print("Unexpected exception:", exc)
+        out = p.stdout.read().decode()
+        if p.returncode == 0:
+            lines = out.splitlines()
+            for line in lines:
+                if line.endswith('Passed\x1b[0m'):
+                    print('✔', end='')
+                elif line.endswith('Skipped\x1b[0m'):
+                    print('…', end='')
+            print()
+        else:
+            print('\r', end='')
+            print(out)
+        return retv | p.returncode
     except EarlyExit:
         return retv
     except FatalError as e:

@asottile
Copy link
Member

asottile commented Mar 2, 2019

I meant more that you'd have a QUIET option there which forwards along --quiet

@rbu
Copy link

rbu commented Apr 26, 2019

I'm a big fan of this, too. Most developers in my team only care about the checks that failed and not those that pass, especially if they are skipped. Can I advocate for a repo-level default for "quiet" (in .pre-commit-config.yaml) and an option (via environment variable?) to override that per user if needed?

@blueyed
Copy link

blueyed commented Apr 26, 2019

@asottile
My solution above was really only a POC to get this going for me with pytest, and it is working well since then.

@asottile asottile removed the question label Jun 9, 2019
@asottile asottile changed the title Question: Is there an option to only output on Failed hook Quiet mode (pre-commit install --quiet) which outputs less while running Jun 9, 2019
@allburov
Copy link

Hey there!
We have a big mono repository with 30 components and every component has his own hooks: mypy, black and reorder-python-imports. We need all of them, because configuration may be different (legacy code base and other causes)

So, when I've changed a one file, have made some mistakes and commit that, wonderful (it's really great tool, thank you!) tool pre-commit get me 89 lines with Skipped and only one with Failed status in the middle of this output.

It's really hard to parse these lines, we must use mouse to scroll to Failed hook. Also when PyCharm or GitKraken is used to commit, they has a small popup, like this:

image

I just want to see the problem or "All is ok" otherwise.

How we can solve this problem? I suggest this way:

  1. MUST Add ability to pass --show: [skipped,failed,passed] to command line interface.
  2. MUST Use env variables PRE_COMMIT_SHOW="failed, passed" pre-commit run
  3. (may be, need to discussion) Add config option to .pre-commit-config.yaml.
show:
  - skipped
  - failed
  - passed

that would change it for all users which I wouldn't want so I don't think that's an appropriate place to change this

But pre-commit already has fail_fast parameter which, as for me, must not apply for all users too. But that's applied, so why not use show also in config?
4. (may be, need to discussion) Add summary information below output:

... a lot of SKIPPED hook
hookname ..... PASSED
hookname ..... FAILED
hookname ..... SKIPPED
hookname ..... SKIPPED
... a lot of SKIPPED hook

Hooks result:
Total: 13 
Passed: 0
Failed: 1
Skipped: 12

for --show=failed,passed it will be

hookname ..... PASSED
hookname ..... FAILED

Hooks result:
Total: 13 
Passed: 0
Failed: 1
Skipped: 12

pre-commit install --quiet isn't good idea, it looks like "You need uninstall\install your software to apply new config". My first reaction is "Oh really, why we could change config in place?"

@asottile If you don't mind for that implementation, I can contribute 1\2 things from my suggestion - supports cli and variables. It will be enough for me and my use case :)

@allburov
Copy link

allburov commented Oct 14, 2019

@asottile
Or we can fully change output, copy it from pytest. @blueyed has show us PoC

pre-commit run # or git commit -m 'Done IS-111'
# . means PASSED
# s means SKIPPED
# F means FAILED
Hooks: ..........Fsssssssssss


# Show a error output only after all
flake8/ptaf-conf-mgr.....................................................Failed
hookid: flake8

services/applications.py:17:1: I100 Import statements are in the wrong order. 'from project.tenants import is_read_only_tenant' should be before 'from project.errors import OperationError'

If we will choose this way, we don't need show option at all.
I would prefer this way :)

But that will be non-backward compatible, may be someone has parse pre-commit output?

@asottile
Copy link
Member

asottile commented Oct 14, 2019

I do not want to drastically change the brand look, so anything like pytest is out.

The most extreme I think I'd be ok with is something like:

Running 89 hooks...........................................Passed

vs

Running 89 hooks...........................................Failed
flake8.....................................................Failed
hookid: flake8

...

but this needs a concrete proposal (configuration value, schema, output scheme, etc.)

@asottile
Copy link
Member

@allburov I don't think the proposed hide_skipped is workable -- it's now impossible to discern the difference between "pre-commit didn't run" and "everything was skipped"

@allburov
Copy link

@asottile you take a words from my mouth.
May be we should add collapse_skipped: True|False (or combine_skipped)? And add summary information at the bottom (with the brand look).

mypy......................................................Passed
flake8.....................................................Failed
hookid: flake8

....

97 of 98 hooks.....................................Skipped

If "everything was skipped" it will be single line:

All hooks.............................................Skipped
# or
98 of 98 hooks....................................Skipped

With pre-commit run -v output will be full.

@asottile
Copy link
Member

I don't think that addresses everyone's desires (and opens the door to 3-4 more similar options which I definitely don't want)

@allburov
Copy link

Do you mean we need go back to disscussion with "universal" solution show_statuses: failed,skipped,passed?

@asottile
Copy link
Member

no I think we need something simpler like quiet: true, maybe something like what I've shown above where there's a single line and then failures if present?

@allburov
Copy link

But how we could show "the progress"? We could use "dot" for show progress, but if we will have 100 hooks - it can occupy two or more lines in output, it's not the brand look.

opens the door to 3-4 more similar options which I definitely don't want

This solution does not resolved this, because someone can ask "Get me passing hooks in quite mode too!"

We should show the progress, of course https://github.com/pre-commit/pre-commit/blob/master/pre_commit/commands/run.py#L111-L112

@asottile
Copy link
Member

we already don't show progress, but it's planned in the future 🤷‍♂

@allburov

This comment has been minimized.

@allburov
Copy link

we already don't show progress, but it's planned in the future

I mean pre-commit show that "Current hook is running, wait". With quite it will look like (with 90 hooks) "All hooks is running wait ... forever?"

@asottile
Copy link
Member

I'm not sure what you want, you either have output, or you're quiet don't have output -- you can't have both

also please don't suggest live patching installed pre-commit, I've had enough troubles with bug reports of dirty installations

@allburov
Copy link

I just want to hide a lot of "Skipped" hooks and have useful output - what hooks are passed or failed.

@asottile
Copy link
Member

I think the argument by others in the thread is that even the Passed ones aren't particularly useful either

@blueyed
Copy link

blueyed commented Oct 16, 2019

You can use a spinner to display progress with many tasks.
FWIW #823 (comment) still works great for me (used with pytest, which does not have many hooks).

@asottile
Copy link
Member

I will not add a spinner and we've already written off that patch

@blueyed
Copy link

blueyed commented Oct 16, 2019

Why is a spinner out of question?
It could be used if there are more tasks than available columns only maybe.

As for the "patch": it's just showing the concept - of course it would not wrap itself in a subprocess when going that route.

@asottile
Copy link
Member

please read the comments above, I've already addressed why a spinner is not ok

@blueyed
Copy link

blueyed commented Oct 16, 2019

@asottile
Well, it's not clear to me otherwise I would have not asked.

Because of the "brand look"?

I do not want to drastically change the brand look, so anything like pytest is out.

(pytest is not using a spinner as you know)

Anyway, I'm out of here - my workaround is OK for me with pytest.. 🤷‍♂️

@allburov
Copy link

allburov commented Oct 19, 2019

@asottile I've reviewed again your solution with "quiet" mode and it looks good for me now. With "quiet" mode we don't have to show progress at all because it's "quiet" mode.

Could we summarize what do you expect?

  1. Add flag --quiet, -q to command pre-commit run, which made output as you suggested above Quiet mode (pre-commit install --quiet) which outputs less while running #823 (comment)
> pre-commit run -q
Running 89 hooks...........................................   # We've run, print welcome message and are waiting all hooks will pass
# With all Passed or all Skipped hooks it will be:
Running 89 hooks........................................... Passed
Running 89 hooks........................................... Skipped 

# If any hook failed, print Failed an hook's output.
Running 89 hooks........................................... Failed

flake8.....................................................Failed
hookid: flake8

...
... git diff 
...

1.1. Introduce env variable PRE_COMMIT_QUIET=0|1 inside pre-commit run, which allow users to enable quiet mode by default in their PC.
2. Add install option --output-mode=[normal|quiet] (with normal by default), which add to hook-tmp option PRE_COMMIT_QUIET = None # True and run pre-commit run with or without --quiet flag.

I've modified your suggestion in the topic with "pre-commit install --quiet", because it makes a little confuse - it looks like we run install process with "quiet" mode, not "install and configure always run with quiet flag"

  1. alternative We can add addopts flag to pre-commit install command, which allow user to add any option (for the future needed). So, I will need run this command to install pre-commit in quiet mode
pre-commit install --addopts="--quite"
# But it's potential point for mistakes and typos

Also, we leave ability to run with "Get full output":

# Suppose, you activated quiet mode when installed pre-commit or set variable in the past
# export PRE_COMMIT_QUIET=1
# pre-commit install --output-mode=quiet
# And have some problem and you have to get full output once

# For manual run
pre-commit run --verbose
# For git command
PRE_COMMIT_QUIET=0 git commit


@asottile
Copy link
Member

seems fine, need to figure out how verbose and quiet interact, let's skip the environment variable for the first implementation, let's not do --addopts it's going to be a footgun (what if someone did --addopts=--files 💥)

@kolinkorr839
Copy link

Just curious what is the workaround for this? I am using version 2.9.3. Is there a patch that I can apply so that I only 'failed' hooks?

@peterjc
Copy link

peterjc commented Mar 20, 2021

@kolinkorr839 you could try my branch on #1560, not looked at this lately to see if there is anything else more recent in progress.

@lucasteligioridis
Copy link

Is a quiet mode an option for this at all? I really do find the output with noise I simply don't care about, I only really care about failures.

It's been over a year since there has been any activity on this issue, I honestly think the option described here is good enough and will eliminate so much noise from the output:
#823 (comment)

@mustafa0x

This comment was marked as off-topic.

@pre-commit pre-commit locked as off-topic and limited conversation to collaborators Mar 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
10 participants