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

Fixed Cookie example from Cookie module #391

Merged
merged 2 commits into from
Apr 5, 2024
Merged

Conversation

tusharad
Copy link
Contributor

@tusharad tusharad commented Mar 27, 2024

Issue Description:

The original example in the Web.Scotty.Cookie module encountered two primary issues that hindered its functionality:

  1. The get function, was not included in the import list from the Web.Scotty module, leading to a missing function error.
  2. The getCookie function was expected to return a Lazy Text type, but instead, it returned a strict Text type. This discrepancy caused a type mismatch with the TL.decimal function, which requires a Lazy Text type.

Screenshot from 2024-03-27 23-53-00

Solution Implemented:

  1. The get function has now been added to the import list within the Web.Scotty.Cookie module.
  2. The code now includes a conversion from strict Text to Lazy Text using the TL.fromStrict function. This conversion aligns with the expected input type for the html function, thereby resolving the type mismatch issue.

Screenshot from 2024-03-27 23-55-09

@ocramz

Copy link
Collaborator

@ocramz ocramz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @tusharad ! Another option would be to import decimal from Data.Text.Read instead.

@tusharad
Copy link
Contributor Author

tusharad commented Mar 28, 2024

Hi @ocramz, I thought about that approach as well.
Scotty's html functions accepts lazy text, we would were gonna have to convert to Lazy varient at the end anyways. Hence I converted it at the begining.

        html $ mconcat [ "<html><body>"
                       , TL.fromStrict hits'
                       , "</body></html>"
                       ]

Let me know which approach you prefer.

@ocramz
Copy link
Collaborator

ocramz commented Apr 2, 2024

Thank you @tusharad ! Your implementation makes a lot of sense. Could you add an entry to the Changelog as well, under Next, please?

@ocramz ocramz self-requested a review April 2, 2024 19:47
@tusharad
Copy link
Contributor Author

tusharad commented Apr 3, 2024

Hello @ocramz, I've just added an entry to the changelog as requested. Do let me know if you need any adjustments. Thanks!

Copy link
Collaborator

@ocramz ocramz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thank you!

@ocramz ocramz merged commit ab5b6b5 into scotty-web:master Apr 5, 2024
6 checks passed
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.

2 participants