evancz / elm-markdown

Markdown parsing within Elm
http://package.elm-lang.org/packages/evancz/elm-markdown/latest
Other
88 stars 7 forks source link

Introduce option for default highlight language #10

Closed jvoigtlaender closed 9 years ago

jvoigtlaender commented 9 years ago

Replaces https://github.com/evancz/elm-markdown/pull/8.

Addresses https://github.com/evancz/elm-markdown/issues/4 (except that not dealing with auto-selection of highlighting language).

Custom code like that in https://github.com/elm-lang/package.elm-lang.org can now call the markdown rendering functions with defaultLanguage in Markdown.Options set to Just "elm" and will achieve the desired result.

evancz commented 9 years ago

Thanks, looks good to me!

evancz commented 9 years ago

I just realized that defaultLanguage is kind of ambiguous. With mardown, you are primarily writing in a human language and it doesn't matter about that, whereas we are talking about the programming language in code blocks. Do you feel that this name is sufficiently precise?

I imagine myself assuming the wrong thing as I learned this API. That said, I can't think of something better. Maybe defaultHighlighting?

What do you think?

jvoigtlaender commented 9 years ago

My code actually originally used the name highlightDefault. I wasn't perfectly happy with that, but yes, defaultLanguage has that other problem, about natural languages. So probably defaultHighlighting is better.

evancz commented 9 years ago

Let's make a list of all the plausible things.

highlightDefault
highlightingDefault
defaultHighlight
defaultHighlighting
defaultLanguage
defaultCodeHighlighting
defaultCodeLanguage
highlighting

I kind of prefer defaultHighlighting though I don't have a very logical basis for that. I think it at least clears up the language vs programming language confusion so it's an improvement.

jvoigtlaender commented 9 years ago

Yes, I think the "default" should come first, and "highlight" or "highlighting" is better than "language" (and also better than "codeLanguage", also because it connects to the name of the relevant JS library used).

jvoigtlaender commented 9 years ago

One other consideration: Anticipating some possible later desires by users, we could also make elm-markdown support highlight.js's auto language selection feature. That is, when no language tag is present, highlight.js could be asked to detect the language.

I'm bringing this up here, because then the relevant option here would have to get a different type than Maybe String, and maybe also a different name?

evancz commented 9 years ago

I kind of don't like that feature of highlight.js. I can't really think of times when I want to host random colorful code, but lets say it is desired. What would the API be then?

type DefaultHighlighting = None | Guess | Default String

Something like that? Seems a bit harder to use than Maybe String and I can't imagine ever using Guess. Can you think of scenarios?

jvoigtlaender commented 9 years ago

Yeah, I don't want to push for this. Let's use the current feature only, and call the options thing "defaultHighlighting".

evancz commented 9 years ago

Cool, made the change.

We can change the type to be more fancy if we someday get a request for language guessing. We'll see if that day comes though :)

evancz commented 9 years ago

I'm in the progress of spinning up a dev version of the package website where we can start test publishing new stuff. I'll email on elm-dev when it is up and I have a few libraries going in there.

I'm hoping to do the release next week, so I want to wait on the release of this until then.

jvoigtlaender commented 9 years ago

Sounds good. Just a "warning": personally, I'll be unresponsive next week.

evancz commented 9 years ago

Okay, thanks for letting me know, and thanks for making all the improvements in the meantime :D