Keep filter when showing unfiltered results on explore page

[JakobDev]

Fixes https://codeberg.org/Codeberg/Community/issues/1302

[silverwind]

BTW, I think this type of query string handling is too prone to bugs as seen many times. We should have a template helper that merges specified params into current URL params:

<a href="{{SetQueryParams ctx "param1" "value1" "param2" "value2"}}"></a>

Would output ?param1=value1&param2=value2 when no params present and ?foo=bar&param1=value1&param2=value2 when foo is already present on current URL.

[JakobDev]

BTW, I think this type of query string handling is too prone to bugs as seen many times.

We both had the same thought. I was actually planning to write a helper of for that next week while working on this. I did not include it in this PR, so it can be backportet.

[silverwind]

The only challenge with such a helper that I see is getting access to the HTTP request in the helper, but I assume if we pass ctx into it, it should work.

I agree to landing this fix as-is.

[wxiaoguang]

It needs QueryEscape. Otherwise, if you search "C++" or "C#", it breaks the URL.

[puni9869]

Its better to do this url formation in backend,

[silverwind]

It needs #27192 (comment)

[puni9869]

but there will be too much if we pass key:value in the template if we have many params. Template will not be human readable. So....

[silverwind]

No, the idea is that every link only specifies the param it controls, so only 1 key/value pair per link. The backend then fills the rest and renders a combined URL with all the existing params present on the request URL from ctx.

[wxiaoguang]

I have no objection if the new approach is well designed. Two points in my mind:

1. $.Link is not necessary. The URL could be as simple as ?k1=v1&k2=v2
2. SetQueryParams doesn't seem a good name. SetXxx seems to be a function which "sets a value to something, has no return value".

[silverwind]

Will make a proof of concept of this soon.

[silverwind]

I think I will name it ExtendQueryParams. Or do you have a better idea?

SetQueryParams doesn't seem a good name. SetXxx seems to be a function which "sets a value to something, has no return value".

At least in JS it is somewhat common, for example a expression foo = 1 also evaluates to the 1 value.

[wxiaoguang]

I think I will name it ExtendQueryParams. Or do you have a better idea?

I also had some thoughts about this problem, since Gitea has a lot of requirements of maintaining the URL components in the template, maybe something like:

// suppose current url is: "/path?k1=v1&k2=v2"
// the "search" matches JS window.location.search, which contains the leading `?`
{{ctx.LinkSearchWith "k1" 123}} -> "?k1=123&k2=v2"
{{WebUtils.BuildSearch "k1" 123}} -> "?k1=123"
{{WebUtils.BuildQuery "k1" 123}} -> "k1=123", it can be used for other "printf"
{{WebUtils.BuildLink "/other-path" (dict "k1" 123)}} -> "/other-path?k1=123"

At least in JS it is somewhat common, for example a expression foo = 1 also evaluates to the 1 value.

No, it is confusing in this case. In your case foo is changed to 1. But in the {{SetQueryParams ctx "only_show_relevant" "0"}} case, has the ctx been changed? That's a wrong design pattern.

[silverwind]

I think ExtendQuery is a good name, based on URL.Query name. Not sure if there is any benefit of making it ctx.ExtendQuery I guess it only worsens performance having to "build" this function every time. I think I prefer ExtendQuery with ctx passed in as first argument.

[wxiaoguang]

Need URL parameter encoding

[wxiaoguang]

Sorry, still need more changes ....

[wxiaoguang]

I haven't tested my suggestion. Let's try.

[wxiaoguang]

It can't be backported to 1.20, IIRC there is no proper ctx support. So the backport/1.20 label is removed.