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

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

Open
stijn-at-work opened this issue Apr 8, 2022 · 6 comments

Comments

@stijn-at-work
Copy link

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
Copy link
Contributor

veewee commented Apr 8, 2022

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 veewee added the question label Apr 8, 2022
@stijn-at-work
Copy link
Author

stijn-at-work commented Apr 8, 2022

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
Copy link
Contributor

veewee commented Apr 8, 2022

@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
Copy link
Author

It works in our office on mac and windows.

@veewee
Copy link
Contributor

veewee commented Apr 8, 2022

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
Copy link
Contributor

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants