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

docs: Fix some typos in helpers/form_helper.rst #8915

Closed
wants to merge 0 commits into from

Conversation

obozdag
Copy link
Contributor

@obozdag obozdag commented May 26, 2024

Description
docs: Fix some typos in helpers/form_helper.rst

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 May 27, 2024
for POST/PUT/DELETE/PATCH requests, but even for GET requests, CSRF filters must be enabled for pages that display Forms.

If you enable CSRF filter with [$globals] https://codeigniter4.github.io/CodeIgniter4/incoming/filters.html#globals), it will be active for all request types.
But if you enable CSRF filter with public array $methods = ['POST' => ['csrf']];, the hidden CSRF field will not be added in GET requests.
Copy link
Member

Choose a reason for hiding this comment

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

Please fix the RST format.

Screenshot 2024-05-31 17 12 12

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kenjis I hope it is fixed now. I don't know how to preview before commit.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Still not good.
Screenshot 2024-06-01 8 19 06

Copy link
Member

Choose a reason for hiding this comment

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

Please apply this patch:

--- a/user_guide_src/source/helpers/form_helper.rst
+++ b/user_guide_src/source/helpers/form_helper.rst
@@ -112,12 +112,17 @@ The following functions are available:
             <form action="http://example.com/index.php/u/sign-up" method="post" accept-charset="utf-8">
             <input type="hidden" id="my-id" name="csrf_test_name" value="964ede6e0ae8a680f7b8eab69136717d">
 
-        .. note:: To use auto-generation of CSRF field, you need to turn on the :ref:`CSRF filter <enable-csrf-protection>` in **app/Config/Filters.php** file.
-            In most cases the form page is requested using the GET method. Normally, CSRF protection is required
-            for POST/PUT/DELETE/PATCH requests, but even for GET requests, CSRF filters must be enabled for pages that display Forms.
-            
-            If you enable CSRF filter with `$globals <https://codeigniter4.github.io/CodeIgniter4/incoming/filters.html#globals>`, it will be active for all request types.
-            But if you enable CSRF filter with public array $methods = ['POST' => ['csrf']];, the hidden CSRF field will not be added in GET requests.
+        .. note:: To use auto-generation of CSRF field, you need to turn on the
+            :ref:`CSRF filter <enable-csrf-protection>` in **app/Config/Filters.php**
+            file. In most cases the form page is requested using the GET method.
+            Normally, CSRF protection is required for POST/PUT/DELETE/PATCH requests,
+            but even for GET requests, CSRF filters must be enabled for pages
+            that display Forms.
+
+            If you enable CSRF filter with :ref:`filters-globals`, it will be
+            active for all request types. But if you enable CSRF filter with
+            ``public array $methods = ['POST' => ['csrf']];``, the hidden CSRF
+            field will not be added in GET requests.
 
     **Adding Hidden Input Fields**
 
diff --git a/user_guide_src/source/incoming/filters.rst b/user_guide_src/source/incoming/filters.rst
index 303f80b551..4425ce03d3 100644
--- a/user_guide_src/source/incoming/filters.rst
+++ b/user_guide_src/source/incoming/filters.rst
@@ -139,6 +139,8 @@ Filters can be specified by adding their alias to either the ``before`` or ``aft
 
 .. literalinclude:: filters/013.php
 
+.. _filters-globals:
+
 $globals
 --------
 

@kenjis
Copy link
Member

kenjis commented Jun 2, 2024

Thank you for updating.
We don't use merge commits in PR branches.
We use git rebase. See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#updating-your-branch
Can you remove the merge commit in the last?

@obozdag
Copy link
Contributor Author

obozdag commented Jun 2, 2024

@kenjis I had problems having my branches in local computer. That is why, when I want to fix something in the guide while reading it on the internet, there is a button "Edit this page", I just click it, edit the file, and commit. This way it creates a new branch each time. After sending form_helper (pacth-9) PR, there was a link on top to update the branch and I clicked. That was wrong as I understand now. But unfortunately I don't know how to roll it back on github. (I work completely on github. No local files or folders on my computer.) So can you please lead me how to do? Or I can delete this patch and close the PR and recreate it.

@kenjis
Copy link
Member

kenjis commented Jun 2, 2024

@obozdag
Have you ever used git command on your local computer?
The detailed workflow is explained in https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md.
Can you follow the documentation?

@obozdag
Copy link
Contributor Author

obozdag commented Jun 4, 2024

@kenjis I followed the documentation. I ran following commands:

  • gh repo clone obozdag/CodeIgniter4
  • cd CodeIgniter4
  • git fetch upstream
  • git switch develop
  • git merge upstream/develop
  • git push origin develop
  • git switch patch-9
  • git rebase upstream/develop
  • git push --force-with-lease origin patch-9

But the PR is automatically closed. So it means I couldn't remove the merge commit in the last?

@kenjis
Copy link
Member

kenjis commented Jun 4, 2024

It seems the patch-9 branch is exactly the same as upstream/develop.
So this PR was closed automatically?

Your commands seem no problem. I'm not sure why both branches are the same.
Did you got an error when you ran git rebase upstream/develop?

@kenjis
Copy link
Member

kenjis commented Jun 5, 2024

I've found your repository'sdevelop is not the same as the upstream.
But I don't know this is the reason why this PR was closed.
Screenshot 2024-06-05 11 01 05

I recommend you make the develop exactly the same as upstream/develop:

$ git checkout patch-9
$ git branch -D develop
$ git checkout --track upstream/develop
$ git push --force-with-lease origin develop

@obozdag
Copy link
Contributor Author

obozdag commented Jun 5, 2024

@kenjis Run those commands. But no difference I think. So I will redo changes from scratch and small changes for each PR this time. Thanks for your effort.

@kenjis
Copy link
Member

kenjis commented Jun 5, 2024

Your https://github.com/obozdag/CodeIgniter4/tree/develop is now up to date with codeigniter4/CodeIgniter4:develop.
But yes, this has nothing to do with the close of this PR.

@kenjis
Copy link
Member

kenjis commented Jun 5, 2024

I uploaded my local patch-9 branch to https://github.com/kenjis/CodeIgniter4/tree/patch-9 for reference.
It may not be the latest one.

@kenjis kenjis mentioned this pull request Jun 5, 2024
5 tasks
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.

2 participants