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

uncheckedHsx + customHsx #2010

Merged
merged 29 commits into from
Nov 2, 2024
Merged

Conversation

kodeFant
Copy link
Contributor

@kodeFant kodeFant commented Oct 26, 2024

I think this works.Probably necessary with eyes of @mpscholten to verify the code is not too crazy. I had some AI help 😄

  • Doesn't parse attribute names or tag names
  • Properly checks for closing tags
  • Still allows for certain tags like <hr> and <br> to not be explicitly closed, but disallows any other so we force users to close everything else. Makes the most sense I think
  • Allows for interpolation of attributes and values

@kodeFant kodeFant marked this pull request as ready for review October 26, 2024 21:14
@kodeFant kodeFant mentioned this pull request Oct 26, 2024
@kodeFant kodeFant marked this pull request as draft October 27, 2024 05:07
@kodeFant
Copy link
Contributor Author

Nah, sorry to say this is not yet great. It seems like the validation stuff works, but the rendering is just awful

@kodeFant
Copy link
Contributor Author

kodeFant commented Oct 31, 2024

@mpscholten, WDYT of latest changes?

  1. I also added tests to ParserSpec. For QQSpec.hs, it seems the only viable option is to duplcate all the testst for uncheckedHsx? Should I do that?

  2. It might be that I gained some courage to attempt the whitelisting thing also now, following the same approach, replacing the checkMarkup with a record type:

data HsxSettings = HsxSettings {
    checkMarkup :: Bool,
    additionalTagNames :: [Text],
    additionalAttributeNames :: [Text]
}

and then for example a custom hsx would look something like this:

customHsx :: [Text] -> [Text] -> QuasiQuoter
customHsx additionalTagNames additionalAttributeNames = QuasiQuoter {
        quoteExp = quoteHsxExpression (HsxSettings True additionalTagNames additionalAttributeNames),
        quotePat = error "quotePat: not defined",
        quoteDec = error "quoteDec: not defined",
        quoteType = error "quoteType: not defined"
    }

Maybe also the additionalTagNames and additionalAttributeNames should be wrapped in a newtype though, but not sure, maybe it's too much:

newtype AdditionalTags = AdditionalTags [Text]
newtype AdditionalAttributes = AdditionalAttributes [Text]

and then

customHsx (AdditionalTags  ["book", "title", "content"]) (AdditionalAttributes ["non-standard", "hrefplusplus"])

@kodeFant
Copy link
Contributor Author

kodeFant commented Oct 31, 2024

I went ahead with the whole thing and added customHsx in the latest commit.

All of what we talked about in #2009 should be solved now.

Example declaration in a separate Helper module:

module Application.Helper.CustomHsx where

import IHP.HSX.QQ (customHsx, AdditionalTags(..), AdditionalAttributes(..))
import Language.Haskell.TH.Quote


myHsx :: QuasiQuoter
myHsx = customHsx 
            (AdditionalTags ["test", "weird"]) 
            (AdditionalAttributes ["strange", "attribute"])

@kodeFant kodeFant changed the title uncheckedHsx uncheckedHsx + customHsx Oct 31, 2024
@kodeFant kodeFant marked this pull request as ready for review October 31, 2024 15:25
ihp-hsx/IHP/HSX/Parser.hs Outdated Show resolved Hide resolved
ihp-hsx/IHP/HSX/QQ.hs Outdated Show resolved Hide resolved
ihp-hsx/IHP/HSX/QQ.hs Outdated Show resolved Hide resolved
ihp-hsx/IHP/HSX/QQ.hs Outdated Show resolved Hide resolved
@kodeFant
Copy link
Contributor Author

Can now be tested like this:

module Application.Helper.CustomHsx where

import IHP.Prelude
import IHP.HSX.QQ (customHsx)
import IHP.HSX.Parser
import Language.Haskell.TH.Quote
import qualified Data.Set as Set

myHsx :: QuasiQuoter
myHsx = customHsx 
                (HsxSettings 
                    { checkMarkup = True,
                      additionalTagNames = Set.fromList ["test", "weird"],
                      additionalAttributeNames = Set.fromList ["strange", "attribute"]
                    }
                )

ihp-hsx/Test/IHP/HSX/CustomHsxCases.hs Show resolved Hide resolved
ihp-hsx/IHP/HSX/QQ.hs Outdated Show resolved Hide resolved
ihp-hsx/IHP/HSX/QQ.hs Outdated Show resolved Hide resolved
Copy link
Member

@mpscholten mpscholten left a comment

Choose a reason for hiding this comment

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

Looks good to me now 👍 Thanks!

@amitaibu
Copy link
Collaborator

amitaibu commented Nov 1, 2024

Thanks! Maybe worth adding a short section in the docs?

@kodeFant
Copy link
Contributor Author

kodeFant commented Nov 1, 2024

@amitaibu Added some docs in the ihp-hsx README and pasted the same text into the IHP Guide

Guide/hsx.markdown Outdated Show resolved Hide resolved
ihp-hsx/README.md Outdated Show resolved Hide resolved
Copy link
Member

@mpscholten mpscholten left a comment

Choose a reason for hiding this comment

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

Thanks 👍

@mpscholten mpscholten merged commit 61bc6a7 into digitallyinduced:master Nov 2, 2024
2 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.

3 participants