CodeIgniter4 icon indicating copy to clipboard operation
CodeIgniter4 copied to clipboard

fix: Update form_helper.php

Open virginus01 opened this issue 1 year ago • 2 comments

Description

This PR enhances the form_open function in the following ways:

  1. Function Existence Check:

    • Added a check to ensure the function form_open is defined only once to avoid redeclaration errors.
  2. Documentation:

    • Included a detailed docblock to describe the function, its parameters, and their types for better understanding and maintenance.
  3. Attribute Consistency:

    • Updated the method attribute to uppercase "POST" and adjusted the check for the 'GET' method to uppercase "GET" to maintain consistency.
  4. CSRF and Charset Handling:

    • Ensured that CSRF and accept-charset attributes are handled consistently.

The updates to the form_open function make it work better under different conditions and make the code easier to read.

Description Explain what you have changed, and why.

Checklist:

  • [x] 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

virginus01 avatar May 18 '24 10:05 virginus01

Thank you for sending this PR!

We expect the following in all Pull Requests (PRs).

[!IMPORTANT] We expect all code changes or bug-fixes to be accompanied by one or more tests added to our test suite to prove the code works.

If pull requests do not comply with the above, they will likely be closed. Since we are a team of volunteers, we don't have any more time to work on the framework than you do. Please make it as painless for your contributions to be included as possible.

See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md

kenjis avatar May 18 '24 11:05 kenjis

@virginus01 Please commit one thing in a git commit.

kenjis avatar May 18 '24 11:05 kenjis

According to https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#attr-fs-method the method value is get or post, not uppercase. So I think lowercase is better.

So, unfortunately, this PR is not acceptable. Also, all PRs must pass all checks by GitHub Action. In the future, please submit PRs that pass the checks. Thank you.

kenjis avatar Jun 06 '24 00:06 kenjis