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

Handle empty indexes for slices and arrays (eg. field[]=1&field[]=2) #64

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mfly
Copy link

@mfly mfly commented May 23, 2024

Fixes Or Enhances

Related to: #60

This PR introduces support for parsing arrays with trailing square brackets (field[]=1&field[]=2).

This format is quite widespread and supported by numerous FE and BE frameworks.

Make sure that you've checked the boxes below before you submit PR:

  • Tests exist or have been written that cover this particular change.

@go-playground/admins

@coveralls
Copy link

coveralls commented May 23, 2024

Coverage Status

coverage: 99.817% (+0.003%) from 99.814%
when pulling fb76283 on mfly:mm-handle-empty-slice-index
into 8785d3c on go-playground:master.

@deankarn
Copy link
Contributor

deankarn commented Jun 1, 2024

@mfly TY for the PR and I would like to support this.

I wonder if this can be abstracted away during parsing within parseMapData if possible though to keep the setFieldByType as simple as possible?

@mfly
Copy link
Author

mfly commented Jun 7, 2024

Hi @deankarn, thanks for expressing your interest in this. I've considered three options:

  1. Preprocess the url.Values by merging keys ending with [] suffix with the respective keys without the suffix - pretty straightforward way to fix the issue, but breaks existing usages like this one
  2. changes in parseMapData - I attempted that, but it seemed out of place and I encountered some problems with namespaces
  3. the proposed approach - pretty simple, explicit and clean. Works with all the use cases I came up with

If you don't like the current approach I can revisit 2. at some point. Happy to take any suggestions you may have.

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