Add ContentTypeForm functionality to DecodeForm #4

syntaqx · 2017-09-01T23:51:32Z

Some of my companies legacy applications require the ability to use ContentTypeForm. I didn't realize go-chi/render didn't support this until we started running into random breakage..

VojtechVitek

Can you show us the use case for this PR so we fully understand it?

We're trying to keep go-chi packages without any unnecessary 3rd party vendor dependencies. I'm not yet sure if we want to pull in gorilla/schema.

syntaqx · 2017-09-02T01:06:01Z

Literally I just want the ability for decoding the following:

$ curl -d "name=test" -X POST http://mysite.com/create

I would say we could spend the time to recreate the functionality of gorilla/schema if you're trying to keep this repository free of dependencies, but it's pretty much the defacto standard for dealing with form data, and has fixed a lot of edge cases around them.

VojtechVitek · 2017-09-02T02:05:33Z

We have couple of options.

syntaqx · 2017-09-02T02:15:21Z

If I'm not mistaken, go-querystring is specifically for the case of utilizing urlencoded query arguments, rather than extracting form data into url.Values.

When comparing ajg/form and gorilla/schema, it seems like both implementations could be viable, it's honestly a flip of the coin on that one.

syntaqx · 2017-09-05T20:42:17Z

@VojtechVitek How would you like me to proceed with this? I believe that gorilla/schema is a good choice for this, but if you would like to vet the others, that's reasonable.

VojtechVitek · 2017-09-05T23:54:29Z

@syntaqx you're right, go-querystring only implements encoding.

We should evaluate decide between the two other packages. Both seem very mature to me.

However, https://github.com/ajg/form looks simpler and more elegant to me.

https://github.com/gorilla/schema looks quite complex for what it's supposed to do, especially the internal cache: https://github.com/gorilla/schema/blob/master/cache.go#L18-L33

syntaqx · 2017-10-31T20:13:37Z

Sorry for the massive delay on this. I've switched to using ajg/form. Please let me know if you have additional concerns.

syntaqx · 2017-11-01T20:47:50Z

@VojtechVitek Let me know if you need anything from me. Would love to switch over to using this package instead of a fork again!

VojtechVitek · 2017-11-02T15:26:34Z

@syntaqx thanks, I'll take a look when I have some spare time. I'm too busy right now with my job

VojtechVitek · 2017-11-28T18:18:05Z

decoder.go

@@ -7,6 +7,8 @@ import (
 	"io"
 	"io/ioutil"
 	"net/http"
+
+	"github.com/ajg/form"


should we bring in github.com/ajg/form as golang/dep dependency?

I think that's a more global question for you, for the project. Currently there's no dependency tooling, so I didn't add any.

otraore · 2018-01-17T13:12:40Z

Any reason as to why this isn't merged yet, we need this at work too.

VojtechVitek · 2018-01-17T16:20:17Z

@otraore I want to get this merged together with golang/dep files.

syntaqx · 2018-01-17T19:28:12Z

I've added dep.

syntaqx · 2018-05-01T11:14:21Z

Any follow up on this?

VojtechVitek · 2018-05-01T16:51:49Z

Sorry I'm too busy at this point. I'll try to come back to this when I have some spare time.

syntaqx · 2018-06-30T15:28:59Z

Hey @VojtechVitek - I know you've been busy, but was just wondering if you've had the chance to look at this? If not, perhaps you'd be willing to add myself or someone else as a maintainer? Would love to be able to keep using this, but without support it's becoming very unreasonable.

syntaqx · 2018-06-30T17:12:34Z

As an aside, gorilla/schema seems to be a better package overall, but I think the general purpose of this PR can use either. Seems that there are occasional reflection issues with ajg/form where it has issues deciphering things, but that can be overridden on a per-application basis, rather than needing to be packaged as the primary option.

SuperFluffy · 2018-09-19T08:30:47Z

Is there some progress on this? I'd love to see this land.

syntaqx · 2018-09-20T18:39:39Z

¯\(ツ)/¯ can't seem to get him to merge it.

VojtechVitek · 2018-09-21T04:19:39Z

Open-source is tough, man. Merging this PR means I'd have to spend hours of testing this changeset in the pre-production & production environments with all of our projects.

I'm willing to do that - but not right away, since I have different priorities at this time.

Send bitcoin or ETH and I'll find some time sooner! :D

syntaqx · 2018-09-24T21:26:10Z

@VojtechVitek While I can appreciate having to find time to deal with a PR on an open-source project, it's literally been a year since I opened this. I'd be happy to manage this project if you're unable to?

VojtechVitek · 2018-09-25T01:29:33Z

You're right. I'm slow. And I'm sorry.

I'll try to test this when I find some time soonish.

syntaqx · 2019-01-30T10:57:54Z

Happy 2019 @VojtechVitek - How're we doing on getting this merged friend? Especially given we have Semantic Versioning now, I feel like the issue is less complicated.

pzeinlinger · 2020-04-05T14:27:14Z

Hey @VojtechVitek,
what about merging this PR? It's not complicated, and we have been running this for quite some time now in production. No issues at all.

FryDay · 2021-04-27T16:14:50Z

Is this project no longer maintained? This is a pretty simple PR and has been open for 4 years.

VojtechVitek · 2021-04-27T19:59:50Z

I'm sorry. I don't currently use go-chi/render.

This PR has some old module mgmt system, so I prefer not to merge it right away.

syntaqx · 2021-04-27T20:17:40Z

I'm more than happy to include Go modules, assuming that you will actually make an effort to get this merged.

What are you using instead of this project, so that people can look to that?

Additionally, I'm more than happy to help maintain this project if you're open to it. It certainly needs a few modernization updates, but there's plenty of participation on the project that could use responses, and some additional features would make it a more than viable option for folks.and would be more than happy to even maintain the repository to provide it with some additional functionality that it would benefit from, b

Removes dep, introduces Go modules, and updates a missing http const

syntaqx · 2021-04-27T20:51:49Z

@VojtechVitek I've updated the pull request:

Uses go modules
Swapped Travis for GitHub Actions (the old tests relied on goimports, and given that I don't use Travis I just swapped to actions instead) - Let me know if you have any concerns here.
Added a couple of everyone's favorite change: build badges in the readme

If we could move to getting this merged in, that would be awesome. Also, please let me know your thoughts on the future of this project, as I would be more than happy to provide it a bit of attention and some much needed updates, but I don't want another 4 years of PR delays ;)

syntaqx · 2021-04-27T20:59:26Z

Sorry, couple minor updates there; didn't realize the linting wasn't passing. Actually good for review/merge now.

FryDay · 2021-04-28T12:25:33Z

I would be willing to assist @syntaqx in maintaining Render. Chi and Render are certainly being used in production in various projects and really need to be kept current.

VojtechVitek

LGTM

Thanks for switching to Go Modules and to Github Actions too. Great job!

For reference:
https://github.com/syntaqx/go-chi-render/runs/2451985236
https://github.com/syntaqx/go-chi-render/runs/2451985149

VojtechVitek · 2021-04-29T13:05:23Z

responder.go

@@ -66,7 +66,7 @@ func PlainText(w http.ResponseWriter, r *http.Request, v string) {
 	if status, ok := r.Context().Value(StatusCtxKey).(int); ok {
 		w.WriteHeader(status)
 	}
-	w.Write([]byte(v))
+	w.Write([]byte(v)) //nolint:errcheck


nit: _ = w.Write() would be imho more idiomatic than using some special comments

VojtechVitek reviewed Sep 2, 2017

View reviewed changes

switching to ajg/form and adding http status constants

580863c

syntaqx changed the title ~~Use gorilla/schema for ContentTypeForm~~ Add functionality for DecodeForm Oct 31, 2017

VojtechVitek mentioned this pull request Nov 3, 2017

Support Content ContentTypeForm #6

Open

VojtechVitek reviewed Nov 28, 2017

View reviewed changes

Merge branch 'master' of github.com:go-chi/render

b6a7960

syntaqx added 2 commits January 17, 2018 12:28

fixing merge conflicts, actually.

17b67c4

really fixing merge conflicts

9fd658c

syntaqx added 3 commits June 30, 2018 09:31

removing vendor folder

a54e4ac

ignoring vendor folder from vcs

bf4aca9

removing boilerplate toml

af1759a

syntaqx changed the title ~~Add functionality for DecodeForm~~ Add ContentTypeForm functionality for DecodeForm Jun 30, 2018

syntaqx changed the title ~~Add ContentTypeForm functionality for DecodeForm~~ Add ContentTypeForm functionality to DecodeForm Jun 30, 2018

syntaqx added 3 commits April 27, 2021 14:23

modernizing DecodeForm changeset

99dff9a

Removes dep, introduces Go modules, and updates a missing http const

Merge branch 'master' of github.com:go-chi/render

126d5e7

swapping travis for github actions

6a082d9

removing minor.patch specificity of actions/cache

56c7eb9

syntaqx requested a review from VojtechVitek April 27, 2021 20:53

adding nolint errcheck until a config can be added

d098851

VojtechVitek approved these changes Apr 29, 2021

View reviewed changes

VojtechVitek merged commit 996350f into go-chi:master Apr 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ContentTypeForm functionality to DecodeForm #4

Add ContentTypeForm functionality to DecodeForm #4

syntaqx commented Sep 1, 2017

VojtechVitek left a comment

syntaqx commented Sep 2, 2017

VojtechVitek commented Sep 2, 2017 •

edited

syntaqx commented Sep 2, 2017

syntaqx commented Sep 5, 2017

VojtechVitek commented Sep 5, 2017 •

edited

syntaqx commented Oct 31, 2017

syntaqx commented Nov 1, 2017

VojtechVitek commented Nov 2, 2017

VojtechVitek Nov 28, 2017

syntaqx Dec 8, 2017

otraore commented Jan 17, 2018

VojtechVitek commented Jan 17, 2018

syntaqx commented Jan 17, 2018

syntaqx commented May 1, 2018

VojtechVitek commented May 1, 2018

syntaqx commented Jun 30, 2018

syntaqx commented Jun 30, 2018

SuperFluffy commented Sep 19, 2018

syntaqx commented Sep 20, 2018 •

edited

VojtechVitek commented Sep 21, 2018 •

edited

syntaqx commented Sep 24, 2018

VojtechVitek commented Sep 25, 2018

syntaqx commented Jan 30, 2019

pzeinlinger commented Apr 5, 2020

FryDay commented Apr 27, 2021

VojtechVitek commented Apr 27, 2021

syntaqx commented Apr 27, 2021

syntaqx commented Apr 27, 2021

syntaqx commented Apr 27, 2021

FryDay commented Apr 28, 2021

VojtechVitek left a comment

VojtechVitek Apr 29, 2021

Add ContentTypeForm functionality to DecodeForm #4

Add ContentTypeForm functionality to DecodeForm #4

Conversation

syntaqx commented Sep 1, 2017

VojtechVitek left a comment

Choose a reason for hiding this comment

syntaqx commented Sep 2, 2017

VojtechVitek commented Sep 2, 2017 • edited

syntaqx commented Sep 2, 2017

syntaqx commented Sep 5, 2017

VojtechVitek commented Sep 5, 2017 • edited

syntaqx commented Oct 31, 2017

syntaqx commented Nov 1, 2017

VojtechVitek commented Nov 2, 2017

VojtechVitek Nov 28, 2017

Choose a reason for hiding this comment

syntaqx Dec 8, 2017

Choose a reason for hiding this comment

otraore commented Jan 17, 2018

VojtechVitek commented Jan 17, 2018

syntaqx commented Jan 17, 2018

syntaqx commented May 1, 2018

VojtechVitek commented May 1, 2018

syntaqx commented Jun 30, 2018

syntaqx commented Jun 30, 2018

SuperFluffy commented Sep 19, 2018

syntaqx commented Sep 20, 2018 • edited

VojtechVitek commented Sep 21, 2018 • edited

syntaqx commented Sep 24, 2018

VojtechVitek commented Sep 25, 2018

syntaqx commented Jan 30, 2019

pzeinlinger commented Apr 5, 2020

FryDay commented Apr 27, 2021

VojtechVitek commented Apr 27, 2021

syntaqx commented Apr 27, 2021

syntaqx commented Apr 27, 2021

syntaqx commented Apr 27, 2021

FryDay commented Apr 28, 2021

VojtechVitek left a comment

Choose a reason for hiding this comment

VojtechVitek Apr 29, 2021

Choose a reason for hiding this comment

VojtechVitek commented Sep 2, 2017 •

edited

VojtechVitek commented Sep 5, 2017 •

edited

syntaqx commented Sep 20, 2018 •

edited

VojtechVitek commented Sep 21, 2018 •

edited