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

Custom banners #237

Closed
Grafikart opened this issue Dec 5, 2016 · 1 comment · Fixed by #238
Closed

Custom banners #237

Grafikart opened this issue Dec 5, 2016 · 1 comment · Fixed by #238
Labels

Comments

@Grafikart
Copy link
Contributor

Q A
Branch master
Bug? yes

I tried to set custom banners but ended up with an error.

My configuration

# grumphp.yml
parameters:
  ascii:
    failed: ko.txt
    succeeded: ok.txt
  git_dir: .
  bin_dir: vendor/bin
  tasks:
    phplint: ~
    phpcsfixer2: ~
    phpunit:
      always_execute: true

Result:

PHP Fatal error:  Uncaught TypeError: Argument 1 passed to GrumPHP\Util\Filesystem::readFromFileInfo() must be an instance of SplFileInfo, string given

In my opinion you should allow string for this method https://github.com/phpro/grumphp/blob/master/src/GrumPHP/Util/Filesystem.php#L19 or convert information from configuration to SplFileInfo

@veewee
Copy link
Contributor

veewee commented Dec 5, 2016

@Grafikart That is indeed a bug that sneeked in with the 0.10.0 release. I've created a patch which will be included in next release. Thanks for noticing!

I'll close this issue for now, but feel free to create a PR for adding an ASCII string as value of the ascii paths. One option is to create an in-memory fileobject:

$file = new SplFileInfo('php://memory');
$content = $file->openFile('r+');
$content->fwrite('put ascii content here');
$content->rewind();

Another thing you could do is just use the string; The hard part will be detecting if the config contains strings or paths. Maybe we could use multiple parameters: one if you want to set the ascii path and another when you want to use an ascii string.
Personally I prefer using a file, but I can image that some people want this in the configuration file.

@veewee veewee closed this as completed Dec 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants