elmcraft / core-extra

Utility functions for an improved experience with elm/core
https://package.elm-lang.org/packages/elmcraft/core-extra/latest/
Other
23 stars 11 forks source link

Fix String.Extra doc comments (use '-->') #30

Closed Janiczek closed 8 months ago

Janiczek commented 1 year ago

This makes the doc tests in String.Extra runnable. Unfortunately it also shows that some of them are failing (see below).

I think the failures are a mix of code being wrong and tests being wrong. Some hammock time might be needed to figure out what the expected behaviour of those functions is supposed to be.

@gampleman Just for transparency, I am not planning to fix these - I just tried the == ➡️ --> change in case it was non-problematic. Ultimately it wasn't, but I hope my commit here will save you or anybody picking up this work, some of the work.

↓ VerifyExamples.String.Extra.SoftWrap2
✗ #softWrap:

    softWrap 7 "My very long text"
    --> "My very\nlong text"

    "My very\nlong text"
    ╷
    │ Expect.equal
    ╵
    "My \nvery \nlong \ntext"

↓ VerifyExamples.String.Extra.SoftWrapWith2
✗ #softWrapWith:

    softWrapWith 7 "..." "My very long text"
    --> "My very...long text"

    "My very...long text"
    ╷
    │ Expect.equal
    ╵
    "My ...very ...long ...text"

↓ VerifyExamples.String.Extra.Unindent1
✗ #unindent:

    unindent "  Hello\n    World "
    --> "Hello\n  World"

    "Hello\n  World"
    ╷
    │ Expect.equal
    ╵
    "Hello\n  World "

↓ VerifyExamples.String.Extra.Wrap1
✗ #wrap:

    wrap 7 "My very long text"
    --> "My very\nlong te\nxt"

    "My very\nlong te\nxt"
    ╷
    │ Expect.equal
    ╵
    "My very\n long t\next"

↓ VerifyExamples.String.Extra.WrapWith1
✗ #wrapWith:

    wrapWith 7 "\n" "My very long text"
    --> "My very\nlong text"

    "My very\nlong text"
    ╷
    │ Expect.equal
    ╵
    "My very\n long t\next"

TEST RUN FAILED

Duration: 514 ms
Passed:   1040
Failed:   5
miniBill commented 10 months ago

@Janiczek tests are failing, can you fix them?

Janiczek commented 10 months ago

@miniBill Sorry, no I won't: Screenshot 2023-11-15 at 15 54 46

Maybe best to close this PR? It served a purpose of finding out what the situation is with the --> doc comments: it's going to be a non-trivial amount of work to fix those.

miniBill commented 10 months ago

Ah, sorry, I had missed that. I think it makes sense to keep it open! Doc tests should not be broken!!!

gampleman commented 8 months ago

I've added a commit that fixes the tests, so I think we can now merge this, right?