Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Stable URL-encoding for Forms #67

Merged
merged 6 commits into from
Jan 30, 2018
Merged

Stable URL-encoding for Forms #67

merged 6 commits into from
Jan 30, 2018

Conversation

fizruk
Copy link
Owner

@fizruk fizruk commented Jan 30, 2018

Closes #49.

@deep-42-thought please verify that this does work on a 32-bit machine.

@deep-42-thought
Copy link

Thanks, this removes some test failures. But I still get

### Failure in /home/vagrant/haskell-http-api-data/src/http-api-data/src/Web/Internal/FormUrlEncoded.hs:254: expression `fromEntriesByKey [("name",["Nick"]),("color",["red","blue"])]'
expected: fromList [("color","red"),("color","blue"),("name","Nick")]
 but got: fromList [("name","Nick"),("color","red"),("color","blue")]
### Failure in /home/vagrant/haskell-http-api-data/src/http-api-data/src/Web/Internal/FormUrlEncoded.hs:405: expression `toEntriesByKey [("name", "Nick"), ("color", "red"), ("color", "white")] :: Either Text [(Text, [Text])]'
expected: Right [("color",["red","white"]),("name",["Nick"])]
 but got: Right [("name",["Nick"]),("color",["red","white"])]
### Failure in /home/vagrant/haskell-http-api-data/src/http-api-data/src/Web/Internal/FormUrlEncoded.hs:537: expression `urlDecodeForm "name=Greg&lastname=Weber"'
expected: Right (fromList [("lastname","Weber"),("name","Greg")])
 but got: Right (fromList [("name","Greg"),("lastname","Weber")])
Examples: 129  Tried: 124  Errors: 0  Failures: 3

Do I read the errors correctly, that a sorting version of fromEntriesByKey, toEntriesByKey and urlDecodeForm is needed, too?

@fizruk
Copy link
Owner Author

fizruk commented Jan 30, 2018

@deep-42-thought not exactly.

toEntriesByKey needs a toEntriesByKeyStable counterpart.
The other two issues are due to Show instance for Form using an unstable toList.

Patch incoming :)

@deep-42-thought
Copy link

thanks @fizruk - as I said, I'm totally new to haskell (I'm just packaging for archlinux32)

@fizruk
Copy link
Owner Author

fizruk commented Jan 30, 2018

@deep-42-thought I've pushed a patch, tested on 64-bit, can you test again on 32-bit?

@deep-42-thought
Copy link

yes, this works! Big thanks!

Running 2 test suites...
Test suite doctests: RUNNING...
Test suite doctests: PASS
Test suite logged to: dist/test/http-api-data-0.3.7.2-doctests.log
Test suite spec: RUNNING...
Test suite spec: PASS
Test suite logged to: dist/test/http-api-data-0.3.7.2-spec.log
2 of 2 test suites (2 of 2 test cases) passed.

@phadej
Copy link
Collaborator

phadej commented Jan 30, 2018

IIRC

sortOn f = sortBy (comparing f)

@fizruk
Copy link
Owner Author

fizruk commented Jan 30, 2018

@phadej is that a problem? :)

@phadej
Copy link
Collaborator

phadej commented Jan 30, 2018

@fizruk for GHC-7.8.4 it seems to be, sortOn is relatively new addition to Data.List.

@fizruk
Copy link
Owner Author

fizruk commented Jan 30, 2018

@phadej right, forgot to check CI, thanks!

@deep-42-thought
Copy link

deep-42-thought commented Jan 30, 2018

if it's of any importance: I can confirm, that it still works on 32 bit

@phadej phadej merged commit b61b152 into master Jan 30, 2018
@phadej phadej deleted the stable-url-encode branch January 30, 2018 13:47
@phadej
Copy link
Collaborator

phadej commented Jan 30, 2018

Merged. @deep-42-thought do you need a release ASAP, or can it wait a little (there should be GHC-8.4 RC very soon, I'd like to hold until that)

@deep-42-thought
Copy link

@phadej there is no need to hurry, thanks!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants