rename context.Query to context.Form

[lunny]

The name Query on context derived from macaron is confusing because it should only get query parameters from URL query.
This PR renames Query to Form as the first step. Next step, we need add the real Query methods and change some routers' revocations.

[coolaj86]

Next step, we need add the real Query methods

Why?

ctx.Req.URL.Query() is easy to understand without having to be familiar with a particular framework.

Why add layers on top of native Go APIs that are already simple, easy to use, well-understood, well-documented, and that have predictable (and desirable) behavior?

Wouldn't it be better to go the opposite direction and remove the legacy Gogs/Macaron/Gitea code in favor of cleaner, simpler, native Go code?
(unless there's a particular missing feature or unexpected caveat in how the native Go code works, of course)

---

I would much prefer to use the standard Go and Chi approaches:

str := r.URL.Query().Get(k)
val := r.FormValue(k)
ctx := r.Context()
data := r.Context().Value("data").(*Data)
tpl := data["something"]

I also understand having a few convenience functions or perhaps an object to hold Data, as it is presently:

r := ctx.Req
str := r.URL.Query().Get(k)
val := r.FormValue(k)
tpl := ctx.Data["something"]

But the idea of adding new APIs that need new documentation around APIs that already work exactly as expected... makes me sad.

[6543]

I thought it will already work as named ?!?

[lunny]

Next step, we need add the real Query methods

Why?

ctx.Req.URL.Query() is easy to understand without having to be familiar with a particular framework.

Why add layers on top of native Go APIs that are already simple, easy to use, well-understood, well-documented, and that have predictable (and desirable) behavior?

Wouldn't it be better to go the opposite direction and remove the legacy Gogs/Macaron/Gitea code in favor of cleaner, simpler, native Go code?
(unless there's a particular missing feature or unexpected caveat in how the native Go code works, of course)

I would much prefer to use the standard Go and Chi approaches:

str := r.URL.Query().Get(k)
val := r.FormValue(k)
ctx := r.Context()
data := r.Context().Value("data").(*Data)
tpl := data["something"]

I also understand having a few convenience functions or perhaps an object to hold Data, as it is presently:

r := ctx.Req
str := r.URL.Query().Get(k)
val := r.FormValue(k)
tpl := ctx.Data["something"]

But the idea of adding new APIs that need new documentation around APIs that already work exactly as expected... makes me sad.

These simple wrap functions will help to convert string to int, bool or others. i.e. QueryInt will try to convert r.URL.Query().Get(k) to int. Since they will be used many places. You have to write strconv.Atoi and above codes everywhere.

[lunny]

I thought it will already work as named ?!?

Like @coolaj86 said QueryXXX should wrap r.URL.Query().Get(k) but not r.Form.Values. So rename Query to Form to make less confusing.

[codecov-commenter]

Codecov Report

Merging #16562 (0d3c9e3) into main (5b2e2d2) will decrease coverage by 0.00%.
The diff coverage is 49.47%.

@@            Coverage Diff             @@
##             main   #16562      +/-   ##
==========================================
- Coverage   45.41%   45.40%   -0.01%     
==========================================
  Files         749      749              
  Lines       84457    84457              
==========================================
- Hits        38353    38348       -5     
- Misses      39928    39932       +4     
- Partials     6176     6177       +1     

Impacted Files	Coverage Δ
modules/context/repo.go	52.99% <0.00%> (ø)
routers/api/v1/admin/adopt.go	0.00% <0.00%> (ø)
routers/api/v1/notify/user.go	11.62% <0.00%> (ø)
routers/api/v1/repo/key.go	17.77% <0.00%> (ø)
routers/api/v1/repo/release_attachment.go	0.00% <0.00%> (ø)
routers/api/v1/repo/topic.go	47.47% <0.00%> (ø)
routers/web/admin/admin.go	11.41% <0.00%> (ø)
routers/web/admin/emails.go	0.00% <0.00%> (ø)
routers/web/admin/hooks.go	0.00% <0.00%> (ø)
routers/web/admin/notice.go	0.00% <0.00%> (ø)
... and 82 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b2e2d2...0d3c9e3. Read the comment docs.

[6543]

This renamed funct. Are used in api a lot so we should migrate them all to use query directly. What porpuse does this form functions do have beside getting query values?!?

@lunny

[6543]

Instead of renaming i propose to refactor it to do what the name sound like or just delete it

[6543]

@lunny i would go this way: #16567

if you are ok with it, I'll finish my pull

[lunny]

@lunny i would go this way: #16567

if you are ok with it, I'll finish my pull

I'm strongly against that PR because it's wrong. Just reviewed that PR, it should take many time. Thanks @6543 .

I agree we need use QueryX for those GET requests, but to make review easier, I would like to make it as two steps. This is the first step to rename QueryX to FormX, next step to add QueryX back and move functions on GET requests to use QueryX .

[6543]

one step at a time ...