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

Add support for markdown statements. #191

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

jsannemo
Copy link
Contributor

@jsannemo jsannemo commented Mar 17, 2021

This PR adds rudimentary support for markdown statements, together with some extra markdown statements for example problems (and a scoring example problem).

Supported:

  • samples, including interactive
  • tables
  • inline math mode using \( \)
  • problem2html

Planned to be added when I add problems that use the feature:

  • footnotes
  • images

Not supported:

  • problem2pdf
  • everything else

This is completely untested in combination with Kattis problem installation, since that's a pain to test locally for me.

@RagnarGrootKoerkamp
Copy link
Contributor

RagnarGrootKoerkamp commented Mar 17, 2021

Nice! We've been considering to add this to the spec as well.

I think it's important to spec this well. Some things that come to mind:

  • I'd prefer to also allow the usual $ delimiters.

  • We should spec whether the first line must be the problem title or not. Latex has \problemname{NAME} on the first line, so I think it makes sense to have # NAME as the first line of a markdown statement.

    • Alternatively, we could render the title from the name in the problem.yaml automatically.
  • Similarly, statements should have \subsection{Input} or the equivalent \begin{Input} ... \end{Input} (which is nice because you can't accidentally mistype Input or put \section instead of \subsection). Some form of checking may be nice. (Or encourage using the latex commands directly, but it's ugly.)

  • I think Pandoc (which I've been trying in BAPCtools for md->tex->pdf rendering) would still support things like $\newcommand$, which is nice to extract common bounds. Does this implementation also do this?

  • For images, one way would be to do it directly via the latex syntax: $\illustration{0.3}{image.jpg}{CC-BY}$. (Ah, but you're probably doing the md->html rendering directly, so maybe it's better to somehow parse the standard markdown image inclusion separately for html/pdf rendering. Sounds non-trivial though.)

@jsannemo
Copy link
Contributor Author

jsannemo commented Mar 17, 2021

Nice! We've been considering to add this to the spec as well.

I think it's important to spec this well. Some things that come to mind:

  • I'd prefer to also allow the usual $ delimiters.

I always felt that syntax in MD to be slightly iffy. In LaTeX escaping $ is part of the syntax, in markdown it is not. You'd have to resort to heuristics to detect math instead, which feels much harder to spec properly.

  • We should spec whether the first line must be the problem title or not. Latex has \problemname{NAME} on the first line, so I think it makes sense to have # NAME as the first line of a markdown statement.

This PR was based on the spec, which suggests that the name entry in problem.yaml should be used instead (and I agree with the spec).

  • Alternatively, we could render the title from the name in the problem.yaml automatically.

That is what I assume the --title option to problem2html does, but I didn't actually check that verifyproblem passes it

  • Similarly, statements should have \subsection{Input} or the equivalent \begin{Input} ... \end{Input} (which is nice because you can't accidentally mistype Input or put \section instead of \subsection). Some form of checking may be nice.

Except for interactive problems, or just problems in general where communication is done in other ways (such as IOI-style with e.g. language specific wrappers). But either way, this feels like something tp be discussed in a new issue.

  • I think Pandoc (which I've been trying in BAPCtools for md->tex->pdf rendering) would still support things like $\newcommand$, which is nice to extract common bounds. Does this implementation also do this?

No

  • For images, one way would be to do it directly via the latex syntax: $\illustration{0.3}{image.jpg}{CC-BY}$. (Ah, but you're probably doing the md->html rendering directly, so maybe it's better to somehow parse the standard markdown image inclusion separately for html/pdf rendering. Sounds non-trivial though.)

The plan is to add a post-processor for img tags that fixes links to images pointing to an image file.

@RagnarGrootKoerkamp
Copy link
Contributor

RagnarGrootKoerkamp commented Mar 18, 2021

Nice! We've been considering to add this to the spec as well.
I think it's important to spec this well. Some things that come to mind:

  • I'd prefer to also allow the usual $ delimiters.

I always felt that syntax in MD to be slightly iffy. In LaTeX escaping $ is part of the syntax, in markdown it is not. You'd have to resort to heuristics to detect math instead, which feels much harder to spec properly.

Hmmm yeah - I'm a bit torn on this. $ is supported by pandoc which is the de facto standard for math in markdown, but probably doesn't have a nice clean spec. I think not allowing $ (and thus parsing $ as normal text) is going to be ugly as well.

Also came across this, which actually argues against \( because that's an escaped parenthesis in plain markdown.

On this topic, we also have to choose a base markdown spec - probably commonmark is the most universal one (as opposed to e.g. github flavored markdown).

  • We should spec whether the first line must be the problem title or not. Latex has \problemname{NAME} on the first line, so I think it makes sense to have # NAME as the first line of a markdown statement.

This PR was based on the spec, which suggests that the name entry in problem.yaml should be used instead (and I agree with the spec).

Hmm where exactly did you read this? This is all tricky since officially the name field in problem.yaml isn't part of the ICPC spec... Although it is what I'm doing for latex as well.
IMO this entire problem name part of the spec could use some cleaning up. Actually I'd prefer to only put the name in the problem.yaml and not again as the first line of the statement, as you currently propose, but the asymmetry with latex statements is annoying.

  • Alternatively, we could render the title from the name in the problem.yaml automatically.

That is what I assume the --title option to problem2html does, but I didn't actually check that verifyproblem passes it

  • Similarly, statements should have \subsection{Input} or the equivalent \begin{Input} ... \end{Input} (which is nice because you can't accidentally mistype Input or put \section instead of \subsection). Some form of checking may be nice.

Except for interactive problems, or just problems in general where communication is done in other ways (such as IOI-style with e.g. language specific wrappers). But either way, this feels like something tp be discussed in a new issue.

See #180 ;) But yeah, it's definitely out of scope to add commands/environments for all possible use cases. But in both BAPC and NWERC we had some potential bugs where people used the wrong type of section heading so I'd like to somehow guard against this.

  • I think Pandoc (which I've been trying in BAPCtools for md->tex->pdf rendering) would still support things like $\newcommand$, which is nice to extract common bounds. Does this implementation also do this?

No

Ack. Actually I'd be ok with having slightly less latex capabilities in a markdown file - if needed you can always revert to the equivalent latex. This means we'd have to make sure that the layout for e.g. lists is exactly equivalent between the two.

  • For images, one way would be to do it directly via the latex syntax: $\illustration{0.3}{image.jpg}{CC-BY}$. (Ah, but you're probably doing the md->html rendering directly, so maybe it's better to somehow parse the standard markdown image inclusion separately for html/pdf rendering. Sounds non-trivial though.)

The plan is to add a post-processor for img tags that fixes links to images pointing to an image file.

Makes sense.

CC @niemela, who has also been wanting/pushing this.

@RagnarGrootKoerkamp
Copy link
Contributor

RagnarGrootKoerkamp commented Mar 18, 2021

For reference, GitLab uses:

$`abc`$

for inline maths (which I don't like so much) and

```math
abc
```

for display math, which is quite nice actually, although I'm not sure whether pandoc does this.

Actually, if we're doing maths, we should definitely support both inline and display maths.

@jsannemo
Copy link
Contributor Author

jsannemo commented Mar 18, 2021 via email

@jsannemo
Copy link
Contributor Author

jsannemo commented Mar 18, 2021 via email

Copy link
Contributor

@pehrsoderman pehrsoderman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all, this is a feature that I will love to get integrated with problemtools!

I gave this a readthrough and gave some general comments regarding style (problemtools isn't quite updated to the style we strive for in the rest of Kattis, but at least type hints and doc strings will make it a bit nicer).

I think it would be nice if @austrin could have a look at it. Also, I think getting the mathmode right is kind of important.

});
</script>
<script type="text/javascript"
src="https://cdnjs.cloudflare.com/ajax/libs/mathjax/2.7.5/MathJax.js?config=TeX-AMS-MML_HTMLorMML">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard-coding links to a CDN is a bit of a maintenance nightmare. This needs to be rethought.

@@ -0,0 +1,65 @@
#! /usr/bin/env python3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file lacks documentation and type hints. It will save us headaches later if that is added.

@@ -8,13 +8,16 @@
from . import template


def convert(problem, options=None):
def convert(problem, options=None, ignore_markdown=False):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add documentation and type hints.

if not os.path.isdir(destdir):
os.makedirs(destdir)

if md2html.get_markdown_statement(problem, options.language):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A warning if both types are present would be nice. Hidden priorities like here are often an annoyance.

@@ -0,0 +1,146 @@
#! /usr/bin/env python3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file lacks documentation and type hints.

Skriv ett program som beräknar differensen mellan icke-negativa heltal.

## Indata
Indatan består av flera testfall (minst \(1\) och som mest \(40\)), ett per rad.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I am overly fond of the \( and \) syntax for math mode in markdown.

@pehrsoderman
Copy link
Contributor

@jsannemo Any progress here?

@pehrsoderman
Copy link
Contributor

@jsannemo Any progress? Also, see comments above...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants