exercism / configlet

The official tool for managing Exercism language track repositories.
https://exercism.org/docs/building/configlet
GNU Affero General Public License v3.0
21 stars 14 forks source link

generate: url targets moved outside of admonition blocks #737

Closed glennj closed 1 year ago

glennj commented 1 year ago

After a configlet generate, this happened: https://github.com/exercism/julia/pull/605/commits/cdc1f82f32b702b5af9f13dce3a4bd75e5e3fb2d#diff-087d9e6fe079ce46da5115c62318d1a0370803e083026468fb1933700bf90b84

The link target was moved from inside an admonition to the end of the file.

I had to fix it manually: https://github.com/exercism/julia/pull/605/commits/4518701ffdbca239588d0bae9cf3bcf9a1f4766b

ee7 commented 1 year ago

Thanks for the report. configlet indeed shouldn't move the link reference definiton in this case.

I was hoping the current markdown parsing would be more temporary than it is. I got most of the way to completing https://github.com/exercism/configlet/issues/628, which would resolve this issue, but I didn't get it over the line.

configlet generate moves the link reference definitions here because that's what we want in the simple case (it also deduplicates them). See the second half of the commit message of https://github.com/exercism/configlet/commit/ef65ef9bcf3ab4c8218e1356d4d9ae83ca235b26 for context. And in particular:

The Markdown handling in this commit is deliberately rudimentary, and has some limitations. We're doing it this way because there is no pure-Nim Markdown parser and renderer that does what we want. The plan is to use a wrapper for libcmark, the reference implementation of the CommonMark Spec.

However, cmark is not designed foremost to be a Markdown formatter - for example, it inlines a link destination from a matching link reference definition. But we want to generate introduction.md files without that inlining, so we need patch or workaround cmark's rendering. There's some complexity there that's best left for another commit.

In hindsight, Markdown's reference link syntax is regrettable:

glennj commented 1 year ago

I figured this would be really hard to fix. Admonitions seem to be handled as nested markdown documents that need a "recursive" parsing solution.

ee7 commented 1 year ago

I think I can fix it for the case you've encountered.

ee7 commented 1 year ago

I've checked that with #738, configlet generate will no longer make a change to the current state of the Julia track.

glennj commented 1 year ago

Awesome! Thank you

ee7 commented 1 year ago

~Hmm. I've noticed that the link still works even when the link reference definition is moved out of the code block. Maybe an earlier version of ee7 knew that.~ Edit: see below.

See exercism/jq/exercises/concept/grade-stats/.docs/introduction.md, which has at the bottom of the file:

[jq-code-add]: https://github.com/stedolan/jq/blob/jq-1.6/src/builtin.jq#L11

because it's been moved from the admonition in exercism/jq/concepts/reduce/introduction.md.

~However, the corresponding link does work on https://exercism.org/tracks/jq/concepts/reduce.~ Edit: the corresponding link indeed doesn't work on https://exercism.org/tracks/jq/exercises/grade-stats/edit

~@ErikSchierboom @glennj Any thoughts on what we want here?~ Edit: presumably we do want them to stay in the block, then.

glennj commented 1 year ago

Where did I see that it was broken...?

I see that that jq file that there is a blank line before the end of the admonition, maybe that's the secret.

How does Exercism render markdown files? Is it something I can do locally without having to commit to a track repo? (probably a question for @ErikSchierboom )

ee7 commented 1 year ago

Oh, sorry - I linked the concept page for reduce, not the introduction file for grade-stats. The introduction is here, and the link is indeed broken:

https://exercism.org/tracks/jq/exercises/grade-stats/edit

ErikSchierboom commented 1 year ago

Admonitions seem to be handled as nested markdown documents that need a "recursive" parsing solution.

They are.

Hmm. I've noticed that the link still works even when the link reference definition is moved out of the code block. Maybe an earlier version of ee7 knew that.

I don't think it will.

edit: ah, you noticed

How does Exercism render markdown files? Is it something I can do locally without having to commit to a track repo? (probably a question for @ErikSchierboom )

We use a Ruby wrapper around commonmarker. If you want to run things locally, you need a couple of dependencies to run, so it's not very easy unfortunately.

ee7 commented 1 year ago

I think we're good here, and can close this issue and create a release.

However I'm suspicious about another edge case: https://github.com/exercism/configlet/issues/742