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

String renderer (WIP) #142

Merged
merged 13 commits into from
Apr 25, 2024
Merged

Conversation

bburdette
Copy link
Contributor

Just implemented the tables today and looks like this:

image

A little hacky with the table heading row and alignment row.

@dillonkearns
Copy link
Owner

Nice @bburdette! One thought I had about a string renderer (can't remember if I've mentioned it to you in the past) is to have a fuzz test that generates different markdown input strings, and then the test case would be something like

expectation randomMarkdown =
let
    parsed = parseMarkdown randomMarkdown
in
    parsed
    |> Expect.equal (parseMarkdown (renderToString parsed))

@bburdette
Copy link
Contributor Author

is there a random markdown generator already, somewhere in the test suite?

@dillonkearns
Copy link
Owner

There isn't one at the moment. Would be quite handy, though. I think it could be pretty barebones and added to little by little.

Here's a starting point for one:

module Fuzzer exposing (..)

import Expect
import Fuzz
import Markdown.Parser
import Test exposing (fuzz)


all =
    fuzz randomMarkdown
        "testOutputs"
        (\randomMarkdownValue ->
            let
                _ =
                    Debug.log "randomMarkdownValue"
                        (randomMarkdownValue
                            |> Markdown.Parser.parse
                        )
            in
            Expect.pass
        )


randomMarkdown : Fuzz.Fuzzer String
randomMarkdown =
    Fuzz.list
        (Fuzz.oneOf
            [ headingFuzzer
            , paragraphFuzzer
            , blockQuoteFuzzer
            ]
        )
        |> Fuzz.map (String.join "\n\n")


paragraphFuzzer : Fuzz.Fuzzer String
paragraphFuzzer =
    Fuzz.list
        (Fuzz.pair Fuzz.bool plainWordFuzzer)
        |> Fuzz.map
            (\pairs ->
                pairs
                    |> List.foldl
                        (\( isNewline, word ) acc ->
                            if isNewline then
                                acc ++ "\n" ++ word

                            else
                                acc ++ " " ++ word
                        )
                        ""
            )


blockQuoteFuzzer : Fuzz.Fuzzer String
blockQuoteFuzzer =
    Fuzz.oneOf
        [ headingFuzzer
        , inlineFuzzer
        ]
        |> Fuzz.map
            (\inlineText ->
                "> " ++ inlineText
            )


headingFuzzer : Fuzz.Fuzzer String
headingFuzzer =
    Fuzz.map3
        (\inlineText level closingHeadingStyle ->
            let
                headingMark =
                    String.repeat level "#"
            in
            headingMark
                ++ " "
                ++ inlineText
                ++ (if closingHeadingStyle then
                        " " ++ headingMark

                    else
                        ""
                   )
        )
        inlineFuzzer
        (Fuzz.intRange 1 6)
        Fuzz.bool


inlineFuzzer : Fuzz.Fuzzer String
inlineFuzzer =
    Fuzz.list
        (Fuzz.oneOf
            [ plainWordFuzzer
            ]
        )
        |> Fuzz.map (String.join " ")


plainWordFuzzer : Fuzz.Fuzzer String
plainWordFuzzer =
    Fuzz.list
        (Fuzz.oneOf
            [ Fuzz.intRange (Char.toCode 'A') (Char.toCode 'Z')
                |> Fuzz.map Char.fromCode
            , Fuzz.intRange (Char.toCode 'a') (Char.toCode 'z')
                |> Fuzz.map Char.fromCode
            ]
        )
        |> Fuzz.map String.fromList

@dillonkearns
Copy link
Owner

Here's a continuation of that from GPT-4 😄

https://gist.github.com/dillonkearns/38d81e5613507237db8b7df727e8ac73

@dillonkearns
Copy link
Owner

Hey @bburdette, not sure if you're still interested in pushing this through. Just wanted to see if there is anything I can do to help, or if you think this is ready to review. Thanks!

@bburdette
Copy link
Contributor Author

Hey! Well its basically ready but I never got around to writing any tests. You are more than welcome to do so if you want, or I might do something with it this week. Would be nice to get it merged.

@bburdette
Copy link
Contributor Author

Ok added the fuzz test, finally. For each random markdown string it tests for parsing/rendering/parsing succeeding.

What it does not do is compare the original string to the parsed-and-rendered string, or compare the parsed Blocks to the rendered-and-parsed Blocks. This is because whitespace doesn't survive the process, so you get:

original string: "# \n\n"

parsed-and-rendered string: "# "

@bburdette
Copy link
Contributor Author

This looks to be failing in the linting step now, "Add elm-review, elm and elm-format to PATH". Something I did?

@dillonkearns
Copy link
Owner

Looking great, thank you @bburdette!

I actually just added the same change the other day on a PR here as well: https://github.com/dillonkearns/elm-markdown/pull/144/files#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fR61

That error with the PATH is because npm bin was removed I guess in a newer version of NPM.

Once you address those elm-review errors this looks good to merge to me! Thank you for working on this!

@dillonkearns
Copy link
Owner

Not sure if you saw by the way, but you can view the elm-review errors inline on this page https://github.com/dillonkearns/elm-markdown/pull/142/files.

@bburdette
Copy link
Contributor Author

bburdette commented Apr 25, 2024

Ha was trying to upgrade elm-review (its too old in nixpkgs) but stuck in nixpkgs version hell.

Anyway yeah will use the inline errors link instead!

@bburdette
Copy link
Contributor Author

Ok basically good to go I think. LMK if anything else!

@dillonkearns dillonkearns merged commit fa2bcda into dillonkearns:master Apr 25, 2024
5 checks passed
@dillonkearns
Copy link
Owner

Amazing, thanks so much for pushing this through @bburdette! 🙏

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