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

Added rm_string in text_helper #3465

Closed
wants to merge 4 commits into from
Closed

Added rm_string in text_helper #3465

wants to merge 4 commits into from

Conversation

mmrtonmoybd
Copy link
Contributor

@mmrtonmoybd mmrtonmoybd commented Aug 8, 2020

Allows to removed specific keyword or fields from string.
I added a new function rm_string in text helper. It will removed specific keywords or fields from given string.
I am not good at English, so I did not give huge description about it.
Thank You All.

Checklist:

  • Securely signed commits
  • Component(s) with PHPdocs
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

Allows to replaced specific keyword or fields from string.
Allows to removed specific keyword or fields from string
@michalsn
Copy link
Member

michalsn commented Aug 8, 2020

The initial issue was related to the validation, but it seems it can be solved with an existing rule.

@mmrtonmoybd mmrtonmoybd changed the title Added is_not_string in text_helper Added rm_string in text_helper Aug 8, 2020
@samsonasik
Copy link
Member

please follow coding standard, you can run composer update and it will auto-fix in next commits when you make new changes.

@michalsn
Copy link
Member

michalsn commented Aug 8, 2020

I'm not really sure if this functionality fits as a helper function. Basically it's a one-liner in a loop... Also, why preg_replace() and not str_replace()?

Personally I'm not sold yet, but maybe you have some good use cases?

Other things:

  • Code should comply with our code style
  • We need tests
  • We need updates to the user guide

Copy link
Contributor Author

@mmrtonmoybd mmrtonmoybd left a comment

Choose a reason for hiding this comment

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

@michalsn I changed the code. Maybe Now it is correct. Review it.

}
return $string;
}

Copy link
Member

@samsonasik samsonasik Aug 9, 2020

Choose a reason for hiding this comment

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

missing close }, also, again, please provide unit tests and follow coding style, do composer update and it will automatically fix it on next commit.

@mmrtonmoybd
Copy link
Contributor Author

mmrtonmoybd commented Aug 9, 2020

@samsonasik
I will fixed this problem soon, also give a unit test.
I close this pull request now, again reopen it soon.
This replace_string are helpful . when the developer didn't show some words/keywords in blog post /comment box.
Example : I want to push drugs in her body.
If I use this function the drugs will be replaced.
Output: I want to push in her body.

Soon I give a new PR .
Thanks to all Codeigniter Teams

@mmrtonmoybd mmrtonmoybd closed this Aug 9, 2020
@michalsn
Copy link
Member

michalsn commented Aug 9, 2020

I just realized that we have word_censor() function already. So this would be redundant functionality.

https://codeigniter4.github.io/userguide/helpers/text_helper.html#word_censor

@mmrtonmoybd
Copy link
Contributor Author

mmrtonmoybd commented Aug 9, 2020

@michalsn Yes the function word_censor is same as my expected. I have not see this function at before.
Sorry for my mistake 😣😣😣.

Codeigniter have not route function in url helper, but codeigniter have site_url and base_url function.
In Laravel https://laravel.com/docs/7.x/helpers#method-route
Have route function .
This is very useful function in view when we working with form.
In Laravel we use like this

<form action="<?= route("blog.post") ?>">

If we didn't not assigned the url in router it's gives a exception.
Codeigniter also need a samilar helper function.
Maybe it will be added soon.

@michalsn
Copy link
Member

No worries, I forgot about this too. I just remembered it because you gave me your use case example.

As for route thing. We have route_to() and url_to() functions:
https://github.com/codeigniter4/CodeIgniter4/blob/develop/system/Common.php#L953
https://github.com/codeigniter4/CodeIgniter4/blob/develop/system/Helpers/url_helper.php#L639

@mmrtonmoybd mmrtonmoybd deleted the is_not_string branch August 10, 2020 08:39
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.

3 participants