Skip to content

docs: fixed typo in view-parser #7844

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

Merged
merged 6 commits into from
Aug 23, 2023

Conversation

sammyskills
Copy link
Contributor

@sammyskills sammyskills commented Aug 22, 2023

Description
Fixed typo in view parser docs for:

  • class reference for renderString()
  • custom filters
  • PHP native functions as filters

Checklist:

  • Securely signed commits
  • [] Component(s) with PHPDoc blocks, only if necessary or adds value
  • [] Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added the documentation Pull requests for documentation only label Aug 22, 2023
@kenjis kenjis mentioned this pull request Aug 22, 2023
22 tasks
@sammyskills
Copy link
Contributor Author

There is something I observed in the docs for PHP native functions as filters...

and its value is any valid native PHP function prefixed with:

The function actually works with/without the backslash \ prefix. Should this be indicated in the docs too?

@paulbalandan
Copy link
Member

The \ there is to indicate you want the native str_repeat function PHP offers. This works in your case with or without the backslash because there are no user-defined function named str_repeat.

@sammyskills
Copy link
Contributor Author

Then, I suggest it should be clearly stated in the docs, something like a "note", maybe something like this:

The \ prefix is required if there is a user-defined function named exactly the same as the PHP native function you want to use as a filter.

WDYT?

@kenjis
Copy link
Member

kenjis commented Aug 22, 2023

The note is more confusing.

a user-defined function named exactly the same as the PHP native function

What do you mean by exactly the same? You cannot define \str_repeat(),
but can define \App\Functions\str_repeat().

All PHP native functions are defined in the global namespace.
So adding \ is harmles, and make sure you call the PHP native function.

@kenjis
Copy link
Member

kenjis commented Aug 22, 2023

No, @sammyskills is correct.

The function actually works with/without the backslash \ prefix.

Yes. In this context, the function name is a callable.

// Filter it....
$replace = $this->config->filters[$filter]($replace, ...$param);

The leading \ is not needed. It is interpreted as a full qualified function name.
See https://3v4l.org/JArO7

@kenjis
Copy link
Member

kenjis commented Aug 22, 2023

The section title PHP Native functions as Filters is not accurate.
https://codeigniter4.github.io/CodeIgniter4/outgoing/view_parser.html#php-native-functions-as-filters
Because it does not matter whether it is a native function or not.

@sammyskills
Copy link
Contributor Author

The section title PHP Native functions as Filters is not accurate.

That means the section will have to be removed completely, and the str_repeat example added to the examples for custom filters.

@kenjis
Copy link
Member

kenjis commented Aug 23, 2023

Yes, all values are PHP callables.

And the first sample code is not good.

    public $filters = [
        'abs'        => '\CodeIgniter\View\Filters::abs',
        'capitalize' => '\CodeIgniter\View\Filters::capitalize',
    ];

\CodeIgniter\View\Filters::abs does not exist.
'capitalize' => '\CodeIgniter\View\Filters::capitalize', are already defined in CodeIgniter\Config\View.
So it is a provided filter, not custom filter.

Copy link
Member

@kenjis kenjis left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you.

Copy link

@TimexPeachtree TimexPeachtree left a comment

Choose a reason for hiding this comment

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

Awesome

@kenjis kenjis merged commit 2c06140 into codeigniter4:develop Aug 23, 2023
@sammyskills sammyskills deleted the doc-view-parser-typo branch August 23, 2023 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Pull requests for documentation only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants