search

nitrogen / simple_bridge

A simple, standardized interface library to Erlang HTTP Servers.
MIT License
112 stars 76 forks source link

Adding field_name to #uploaded_file record #19

Closed ngaranko closed 11 years ago

ngaranko commented 11 years ago

There were no way to determinate which field name represented by this or another file. Now fixed.

ngaranko commented 11 years ago

Actually my last commit removes #uploaded_file record from Req:post_files() and transfers post_files() from list into proplist. Discussion is here: https://groups.google.com/forum/?fromgroups=#!topic/chicagoboss/RhfRgr6QmqQ

Not sure if it's totally good idea, please let me know if I should remove this changes from pull request.

choptastic commented 11 years ago

There is definitely value in what you present here. It makes sense to be able to retrieve the field name.

That said, I'm not super keen on the return being a proplist - the free nature of proplists are great, but don't replace the hard rigidity of the records.

What I am very open to is the idea of adding an uploaded_file.erl module that could then be getters for the uploaded_file record. That way, apps using the simple_bridge API such as Nitrogen and ChicagoBoss could have more clearly defined accessing patterns than either doing records (which are clunky to include from a dependency) or proplists (which are not clearly defined).

And at the same time, it would retain backwards compatibility with anyone who might be working with the #uploaded_file record directly for whatever reason.

ALSO, for you ChicagoBoss guys who like your pmods, it could very well be used as a pmod (ie UploadedFile:field_name()). If my baby falls asleep in the next 15-30 minutes, I'll try to whip something up for this.

Overall, a uploaded_file module would provide a clearly defined and superior interface to the uploaded files.

choptastic commented 11 years ago

Part of your pull request has been cherry-picked and can be found here: https://github.com/choptastic/simple_bridge/commit/1c79cb1d1bd17b766229b793ea8fd511ecb8de4f

The API I mentioned has been added and can be found here: https://github.com/choptastic/simple_bridge/commit/11935b91fbafd7a3525b02f9a2bbd7177449944f

I'll be pushing these changes to mainline soon.

Thanks for the pull request!