grumphp icon indicating copy to clipboard operation
grumphp copied to clipboard

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

Open stijn-at-work opened this issue 2 years ago • 10 comments

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) ✗

stijn-at-work avatar Apr 08 '22 08:04 stijn-at-work

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.

veewee avatar Apr 08 '22 08:04 veewee

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)

stijn-at-work avatar Apr 08 '22 09:04 stijn-at-work

@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

veewee avatar Apr 08 '22 09:04 veewee

It works in our office on mac and windows.

stijn-at-work avatar Apr 08 '22 09:04 stijn-at-work

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.

veewee avatar Apr 08 '22 17:04 veewee

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)

DennisdeBest avatar Aug 29 '22 10:08 DennisdeBest