search

greghendershott / markdown

Markdown parser written in Racket.
101 stars 28 forks source link

`xexprs->scribble-pres` doesn't work on `...` #89

Closed KDr2 closed 2 months ago

KDr2 commented 2 months ago

Reproduce:

Evaluate (xexprs->scribble-pres (parse-markdown "...")), then we will get:

> (xexprs->scribble-pres (parse-markdown "..."))
para: contract violation
  expected: pre-content?
  given: '(hellip)
  in: an element of
      the rest argument of
      (->*
       ()
       (#:style (or/c style? string? symbol? #f))
       #:rest
       (listof pre-content?)
       paragraph?)
  contract from:
      <pkgs>/scribble-lib/scribble/base.rkt
  blaming: <pkgs>/markdown/markdown/scrib.rkt
   (assuming the contract is correct)
  at: <pkgs>/scribble-lib/scribble/base.rkt:318:2
 [,bt for context]
greghendershott commented 2 months ago

Caveat: I'm rusty on this markdown package, as well as Scribble details.

The core problem seems to be that, although Scribble supports some HTML entity symbols, hellip isn't one, which I hadn't realized.

  1. Generally: To avoid any contract failures, I should test symbols for content? before just passing them through as-is. Otherwise I can idk just convert the symbols to a string, so something at least appears in the output (?), as a general fallback.

  2. As for ellipses specifically, I could special case 'hellip to convert back to "..." plain text, and let the Scribble decoder do whatever it will with that, to make fancier ellipses.

Something like the following seems to work in initial quick experiments:

@@ -75,7 +76,10 @@
       [(? string? s) s]
-      [(? symbol? s) s]
+      [(? symbol? s)
+       (cond [(content? s) s] ;issue #89
+             [(eq? s 'hellip) "..."]
+             [else (format "~a" s)])]

But maybe I'll sleep on that, and wait to see if you have any ideas/suggestions, before merging a commit?

KDr2 commented 2 months ago

The fix is pretty clear and works well, Thank you!

greghendershott commented 2 months ago

@KDr2 Great! I'll merge the fix after CI tests finish running. (As with most of my repos, I have this set on GitHub so I can't push to the main branch unless tests pass.)

I reopened the issue because the fix isn't merged yet; it will auto-close when merged.

Thanks again for the report!