Skip to content

Update fileinfo names #6206

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

Closed
wants to merge 1 commit into from
Closed

Update fileinfo names #6206

wants to merge 1 commit into from

Conversation

nikic
Copy link
Member

@nikic nikic commented Sep 24, 2020

Normalizing $file_name to $filename, $options -> $flags (which works particularly well here, because the function names actually use "flags") and $arg -> $magic_file (following docs here).

I first changed $context to $stream_context, but then changed it back, because we use $context everywhere else. The only usage of $stream_context is in hash_update_file(), which already has a different $context parameter.

Copy link
Member

@kocsismate kocsismate left a comment

Choose a reason for hiding this comment

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

Looks good! Maybe the $magic_file param name could be clarified by including database somewhere, but I don't think it's a serious problem to solve.

@nikic
Copy link
Member Author

nikic commented Sep 24, 2020

Looks good! Maybe the $magic_file param name could be clarified by including database somewhere, but I don't think it's a serious problem to solve.

What do you think about calling it $magic_database?

@kocsismate
Copy link
Member

What do you think about calling it $magic_database?

Exactly what I had in mind :) (besides $magic_database_filename, but that's too long for sure).

}

/** @return resource|false */
function finfo_open(int $options = FILEINFO_NONE, string $arg = "") {}
function finfo_open(int $flags = FILEINFO_NONE, string $magic_database = "") {}
Copy link
Member

Choose a reason for hiding this comment

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

perfect! :)

@php-pulls php-pulls closed this in 25b9ba8 Sep 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants