#67 Conditional Inclusion of catsoop Source

Merged
hz merged 7 commits from :conditional_markdown into master 1 month ago
hz commented 1 month ago

This is a partial implementation of one of the ideas discussed in #56 (ages ago) about conditionally including catsoop source. This implements the third option @edemaine originally presented:

<div cs-hide-if="@{condition}"> or <div cs-show-if="@{condition}"> or other HTML attributes that trigger Catsoop action (deleting element if/unless Python condition holds). Perhaps the simplest approach? Again relying on HTML-in-Markdown, which can (in appropriate circumstances, with enough blank lines as I recall) support Markdown inside HTML blocks.

So this does now support things like the following:

<div cs-show-if="@{role == 'Instructor'}">

*Greetings*, Professor!
</div>

with a couple of limitations.

Most notably, this doesn’t really align with either the original Markdown spec, nor the CommonMark spec, for handling HTML blocks. In particular, it only does the blank-line trick on div tags as written, and only for a blank line immediately following the div tag.

So it’s a big gross hack...but it kind of allows conditional inclusion of a p, div, or span via cs-hide-if and cs-show-if attributes.

@edemaine, not sure if you have any thoughts on this as written?

This is a partial implementation of one of the ideas discussed in #56 (ages ago) about conditionally including catsoop source. This implements the third option @edemaine originally presented: > `<div cs-hide-if="@{condition}">` or `<div cs-show-if="@{condition}">` or other HTML attributes that trigger Catsoop action (deleting element if/unless Python condition holds). Perhaps the simplest approach? Again relying on HTML-in-Markdown, which can (in appropriate circumstances, with enough blank lines as I recall) support Markdown inside HTML blocks. So this does now support things like the following: ``` <div cs-show-if="@{role == 'Instructor'}"> *Greetings*, Professor! </div> ``` with a couple of limitations. Most notably, this doesn't really align with either the original Markdown spec, nor the CommonMark spec, for handling HTML blocks. In particular, it only does the blank-line trick on `div` tags as written, and only for a blank line _immediately_ following the `div` tag. So it's a big gross hack...but it kind of allows conditional inclusion of a `p`, `div`, or `span` via `cs-hide-if` and `cs-show-if` attributes. @edemaine, not sure if you have any thoughts on this as written?
hz added this to the 2020.9.0 milestone 1 month ago
hz added the
enhancement
label 1 month ago
edemaine commented 1 month ago

Definitely like the simple implementation of cs-hide-if and cs-show-if! It’s been too long, but I assume that the @{...} processing happens elsewhere. I don’t remember coming up with the <div cs-show-if="@{...}"> notation -- it does seem like a lot of punctuation, but perhaps all necessary. In particular, I guess those quotes are necessary in case the expression expands to empty string.

It’s sad that you have to hack part of the CommonMark spec onto Python-Markdown. I just found this fun comparison tool:

https://johnmacfarlane.net/babelmark2/?text=%3Cdiv%3E%0A%0A%60hi%60%0A%0A%3C%2Fdiv%3E%0A

Output in case that site goes down:

image

From Coauthor I’m familiar with (JS-based) markdown-it, which claims to follow CommonMark spec. Maybe a little crazy to change Markdown parsers (can’t imagine what tiny things would change), but if that’s an option, perhaps worth considering for this? Here’s a list.

Definitely like the simple implementation of `cs-hide-if` and `cs-show-if`! It's been too long, but I assume that the `@{...}` processing happens elsewhere. I don't remember coming up with the `<div cs-show-if="@{...}">` notation -- it does seem like a lot of punctuation, but perhaps all necessary. In particular, I guess those quotes are necessary in case the expression expands to empty string. It's sad that you have to hack part of the CommonMark spec onto Python-Markdown. I just found this fun comparison tool: https://johnmacfarlane.net/babelmark2/?text=%3Cdiv%3E%0A%0A%60hi%60%0A%0A%3C%2Fdiv%3E%0A Output in case that site goes down: ![image](/git/attachments/bda44e24-7633-45bb-b765-7cc90850a187) From Coauthor I'm familiar with (JS-based) markdown-it, which claims to follow CommonMark spec. Maybe a little crazy to change Markdown parsers (can't imagine what tiny things would change), but if that's an option, perhaps worth considering for this? [Here's a list](https://github.com/commonmark/commonmark-spec/wiki/list-of-commonmark-implementations#python).
177 KiB
hz commented 1 month ago
Owner

It’s sad that you have to hack part of the CommonMark spec onto Python-Markdown.

Agreed. I did a very little bit of looking into this last night, and the first couple of implementations I found did not look particularly extensible (to the point where adding the math stuff math felt like it might be a little bit painful).

I’m not at all opposed to changing Markdown parsers. Thanks for sharing the list; I’ll take a look at those and see if anything fits the bill!

> It's sad that you have to hack part of the CommonMark spec onto Python-Markdown. Agreed. I did a _very_ little bit of looking into this last night, and the first couple of implementations I found did not look particularly extensible (to the point where adding the math stuff math felt like it might be a little bit painful). I'm not at all opposed to changing Markdown parsers. Thanks for sharing the list; I'll take a look at those and see if anything fits the bill!
hz commented 1 month ago
Owner

It’s been too long, but I assume that the @{...} processing happens elsewhere.

...it does seem like a lot of punctuation, but perhaps all necessary. In particular, I guess those quotes are necessary in case the expression expands to empty string.

Right. @{} is handled earlier, to evaluate Python code.

An alternative implementation, which is nicer by some metrics and worse by others, would be to make that implicit, and always to evaluate the thing in cs-hide-if="some_expression". I think that’s possible, and I think it might be nicer. I’ll play with that a little bit.

I do think having a proper CommonMark parser would make things easier here, too, so I’ll look into that first.

> It's been too long, but I assume that the `@{...}` processing happens elsewhere. > ...it does seem like a lot of punctuation, but perhaps all necessary. In particular, I guess those quotes are necessary in case the expression expands to empty string. Right. `@{}` is handled earlier, to evaluate Python code. An alternative implementation, which is nicer by some metrics and worse by others, would be to make that implicit, and always to evaluate the thing in `cs-hide-if="some_expression"`. I think that's possible, and I think it might be nicer. I'll play with that a little bit. I do think having a proper CommonMark parser would make things easier here, too, so I'll look into that first.
hz commented 1 month ago
Owner

OK, the switch to mistletoe for handling CommonMark was pretty painless. There are a few rendering issues on 6.009 pages, but those all appear to be because we were doing the wrong thing (to make it somehow match what catsoop was spitting out before), and not because of a problem with mistletoe AFAICT.

I had to make one slight change to the tests in order for them to pass, but I don’t think it’s anything to worry about. mistletoe strips whitespace from the front of lines, so the precise output isn’t the same as it used to be, but since TeX ignores whitespace on the front of lines anyway (@edemaine, you can correct me if I’m wrong on that), I don’t expect any issues.

I also changed cs-hide-if and cs-show-if so that they don’t need @{}; i.e., they evaluate their values automatically.

I’m going to go ahead and merge this in under the assumption that this is a good idea on the whole, and we can fix issues later (@edemaine, if you have any other feedback along these lines, let me know; and thanks for the help/suggestion here!).

OK, the switch to [mistletoe](https://github.com/miyuchina/mistletoe) for handling CommonMark was pretty painless. There are a few rendering issues on 6.009 pages, but those all appear to be because we were doing the wrong thing (to make it somehow match what catsoop was spitting out before), and not because of a problem with `mistletoe` AFAICT. I had to make one slight change to the tests in order for them to pass, but I don't think it's anything to worry about. `mistletoe` strips whitespace from the front of lines, so the precise output isn't the same as it used to be, but since TeX ignores whitespace on the front of lines anyway (@edemaine, you can correct me if I'm wrong on that), I don't expect any issues. I also changed `cs-hide-if` and `cs-show-if` so that they don't need `@{}`; i.e., they evaluate their values automatically. I'm going to go ahead and merge this in under the assumption that this is a good idea on the whole, and we can fix issues later (@edemaine, if you have any other feedback along these lines, let me know; and thanks for the help/suggestion here!).
hz closed this pull request 1 month ago
edemaine commented 1 month ago

Great!

mistletoe strips whitespace from the front of lines, so the precise output isn’t the same as it used to be, but since TeX ignores whitespace on the front of lines anyway (@edemaine, you can correct me if I’m wrong on that), I don’t expect any issues.

That’s correct on the TeX front. Surely preformatted text with backticks doesn’t have its leading whitespace stripped though? Or is this a plugin thing?

I also changed cs-hide-if and cs-show-if so that they don’t need @{}; i.e., they evaluate their values automatically.

Cool! Should be really helpful!

Great! > `mistletoe` strips whitespace from the front of lines, so the precise output isn't the same as it used to be, but since TeX ignores whitespace on the front of lines anyway (@edemaine, you can correct me if I'm wrong on that), I don't expect any issues. That's correct on the TeX front. Surely preformatted text with backticks doesn't have its leading whitespace stripped though? Or is this a plugin thing? > I also changed `cs-hide-if` and `cs-show-if` so that they don't need `@{}`; i.e., they evaluate their values automatically. Cool! Should be really helpful!
hz commented 1 month ago
Owner

That’s correct on the TeX front. Surely preformatted text with backticks doesn’t have its leading whitespace stripped though? Or is this a plugin thing?

Right. I wasn’t quite explaining the issue properly there. Certain things are implemented as line-based processors, and newlines are removed as part of that process. Since code blocks (indented or fenced) are handled as part of that process, they don’t have their leading whitespace stripped.

But because all of the math processors (even those that can be split across multiple lines) could be on a single line, I didn’t think it made sense to handle them during that part of the process, so they are all implemented as “span tokens,” which, as I understand it, are processed after all of the line-based processors are done.

In theory, it’s probably possible to stick these in at a point where they happen before the leading whitespace has been stripped. But I’m not sure it’s worth the trouble, since this should produce equivalent output for the math elements.

> That's correct on the TeX front. Surely preformatted text with backticks doesn't have its leading whitespace stripped though? Or is this a plugin thing? Right. I wasn't quite explaining the issue properly there. Certain things are implemented as line-based processors, and newlines are removed as part of that process. Since code blocks (indented or fenced) are handled as part of that process, they don't have their leading whitespace stripped. But because all of the math processors (even those that can be split across multiple lines) _could_ be on a single line, I didn't think it made sense to handle them during that part of the process, so they are all implemented as "span tokens," which, as I understand it, are processed after all of the line-based processors are done. In theory, it's probably possible to stick these in at a point where they happen before the leading whitespace has been stripped. But I'm not sure it's worth the trouble, since this should produce equivalent output for the math elements.
hz deleted branch conditional_markdown 1 month ago
The pull request has been merged as 3bd9e0fa7a.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.