phpro / grumphp

A PHP code-quality tool
MIT License
4.11k stars 430 forks source link

I can fix some stuff automatically, do you want me to? (yes/no) [no]: > % #1000

Open stijn-at-work opened 2 years ago

stijn-at-work commented 2 years ago
Q A
Version 1.5.1
Bug? yes
New feature? no
Question? no
Documentation? yes/no
Related tickets comma-separated list of related tickets

I would like to fix the coding standard issues interactively by answering the question 'I can fix some stuff automatically, do you want me to? (yes/no) [no]:' with yes. However, the terminal prefills the question with % and ends the conversation.

This blog article points explains that the interaction should work as i expect. https://veewee.github.io/blog/let-grumphp-fix-your-code/

My configuration

grumphp:
    ascii:
        failed: ~
        succeeded: ~
    fixer:
        enabled: true
        fix_by_default: false
    tasks:
        phplint: ~
        phpstan:
            # no need to set level here, it is read from phpstan.neon
            level: ~
        phpcs:
            standard:
                - 'PSR12'

Steps to reproduce:

# Your actions
- created a test.php file with a coding standard error to trigger phpcs

# Run GrumPHP:
git add -A && git commit -m"Test"

Result:

➜  baseplatform git:(develop) ✗ gcmsg 'test'
GrumPHP detected a pre-commit command.
GrumPHP is sniffing your code!

Running tasks with priority 0!
==============================

Running task 1/3: phplint... ✔
Running task 2/3: phpstan... ✔
Running task 3/3: phpcs... ✘

phpcs
=====

FILE: /Applications/XAMPP/xamppfiles/htdocs/baseplatform/test.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 3 | ERROR | [x] Opening brace should be on a new line
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Time: 62ms; Memory: 8MB

You can fix errors by running the following command:
'/Applications/XAMPP/xamppfiles/htdocs/baseplatform/vendor/bin/phpcbf' '--standard=PSR12' '--extensions=php' '--report=full' '/Applications/XAMPP/xamppfiles/htdocs/baseplatform/test.php'
To skip commit checks, add -n or --no-verify flag to commit command

 I can fix some stuff automatically, do you want me to? (yes/no) [no]:
 > %                                                                            
➜  baseplatform git:(develop) ✗
veewee commented 2 years ago

Hello @stijn-at-work

The article has this note:

Note: The fix_by_defaut parameter will also be used in situations where CLI input is not supported. Depending on your CLI, this could be e.g. during pre-commit.

During pre-commit, the git diff is being piped into grumphp, meaning thet grumphp runs in non-interactive mode and you are not able to give an answer the question during pre-commit.

I haven't found a solution for that problem yet. Not sure if it is fixable alltogether: Imagine you commit from a UI, you won't be able to answer either anyways.

stijn-at-work commented 2 years ago

i think i have a solution: i add <dev/tty in pre-commit hook

cd .git/hooks vim pre-commit add '< /dev/tty'

#!/bin/sh

#
# Run the hook command.
# Note: this will be replaced by the real command during copy.
#

# Fetch the GIT diff and format it as command input:
DIFF=$(git -c diff.mnemonicprefix=false -c diff.noprefix=false --no-pager diff -r -p -m -M --full-index --no-color --staged | cat)

# Grumphp env vars

export GRUMPHP_GIT_WORKING_DIR="$(git rev-parse --show-toplevel)"

# Run GrumPHP
(cd "./" && printf "%s\n" "${DIFF}" | exec 'vendor/bin/grumphp' 'git:pre-commit' '--skip-success-output' < /dev/tty)
veewee commented 2 years ago

@stijn-at-work cool!

Not sure if that will work on all systems and in all situations. But If it does, I see no issue in adding it!

This requires some additional testing

stijn-at-work commented 2 years ago

It works in our office on mac and windows.

veewee commented 2 years ago

Nice. Will also have to make sure it works on docker setups.

Doesn't it overwrite the stdin stream and therefore skips the diff that if being piped into it? Meaning it won't be able to detect the diff of partial commits anymore.

Just thinking out loud: Maybe it is an option to provide a git hook variable that allows you to either use the diff or stdin as input Maybe it is possible to detect partial commits and change behaviour based on that.

DennisdeBest commented 1 year ago

Thanks @stijn-at-work adding < /dev/tty fixed it for me.

This is my grumphp.yaml.dist

grumphp:
  hooks_dir: './tools/grumphp/resources/hooks'
  git_hook_variables:
    EXEC_GRUMPHP_COMMAND: 'docker-compose run  -eSYMFONY_DEPRECATIONS_HELPER=disabled=1 php'
  tasks:
  ...

I added my custom hooks, this is the /tools/grumphp/resources/hooks/pre-commit

  #!/bin/sh

DIFF=$(git -c diff.mnemonicprefix=false -c diff.noprefix=false --no-pager diff -r -p -m -M --full-index --no-color --staged | cat)

# Grumphp env vars
$(ENV)
export GRUMPHP_GIT_WORKING_DIR="$(git rev-parse --show-toplevel)"

# Run GrumPHP
(cd "${HOOK_EXEC_PATH}" && printf "%s\n" "${DIFF}" | $(EXEC_GRUMPHP_COMMAND) $(HOOK_COMMAND) '--ansi' '--skip-success-output' < /dev/tty)