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

Send file contents #134

Merged
merged 6 commits into from
Jan 13, 2021
Merged

Send file contents #134

merged 6 commits into from
Jan 13, 2021

Conversation

patinthehat
Copy link
Contributor

This PR adds the file() method, which accepts a filename as its only parameter and sends the contents of the file to Ray. This method also sends a label containing the filename along with the payload.
Note: The file content is encoded with htmlentities() and nl2br() to ensure HTML files are displayed correctly.

Developers will find this addition very helpful when working with the file system and need to debug multiple files quickly.

Unit tests and documentation have been added.


public function getContent(): array
{
if (!file_exists($this->filename)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you rewrite this without the else statement? This branch could just return an array with the properties that should be set when the file does not exists. I also don't mind this case using the label File.

Could you also add this method to the framework agnostic usage docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you say "this case using the label "File", do you mean only the case where the file does not exist? or when it does exist - or both?

I assume you mean adding file() to the docs - I already did, see the "Working with files" section I added above. I didn't add a screenshot as the other screenshots are from MacOS and I'm running Linux, so it'd have to be redone anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requested changes submitted.

src/Ray.php Outdated Show resolved Hide resolved
@freekmurze
Copy link
Member

Could you make sure that all tests pass?

@patinthehat
Copy link
Contributor Author

Could you make sure that all tests pass?

Absolutely. All tests are now passing.

@freekmurze freekmurze merged commit c401fab into spatie:master Jan 13, 2021
@freekmurze
Copy link
Member

Thank you!

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

Successfully merging this pull request may close these issues.

2 participants