Closed cjw296 closed 12 months ago
what have you done Chris /o\
GFM is basically a subset of MyST, as best I can tell, so it's largely the documentation, but also parsers that won't pick up the %-style
comments that MyST uses.
Okay, and the docs for this PR are here, chicken and egg with creating the PR.
There's one thing to get out of the way first: inline HTML comments are in no way specific to GFM. Inline HTML is even part of Gruber's original specification: https://daringfireball.net/projects/markdown/syntax#html
That means it should perfectly work with MyST too. Is there anything else specific to GFM in the implementation? Can't find anything on a first look.
As I said, GFM is a subset of MyST, so sure, inline html comments work fine with the Myst parsers, they're even in the docs ;-) (I actually fixed up a lot of this in https://github.com/simplistix/sybil/pull/94, which landed this week)
It's more that these docs are focused on folks who aren't using MyST, and who may not even know what MyST is.
Yeah but what I mean is: what is actually GFM about your GFM support? It looks like vanilla Markdown to me?
I don't know, you tell me? :-) (I don't really use Markdown, which is why I want you to review this PR!) My thought process is:
document "Markdown Parsers", but be clear that these are targeting GFM. I don't think there's anything GFM specific (unless you tell me there needs to be!)
document "MyST Parsers", and they do have a bunch of implementation, since MyST adds a bunch of ReST-ish stuff on top of Markdown.
@hynek - last call on this, would appreciate your review on it but I'm planning a 6.0.0 release sometime in the next day or few and I intend to land this PR for it...
I have scrolled through the docs in the PR and my feedback remains the same: unless I'm missing anything, all you're doing is looking into <--! FOO -->
in addition to % FOO
.
If that's indeed what you're doing, you shouldn't call it GFM. Just call it sybil.markdown
or sybil.commonmark
. Otherwise you'll detract/irritate users of platforms that also support Markdown but not the GitHub-flavored kind. Just say "Markdown, including GitHub-flavored Markdown (GFM), ..." and call it a day?
As for the contents (again, only judging the docs), as long as I can replace my multi-line skips with <--! skip -->
I'm a happy camper.
And yes, I try to avoid having doctests within docstrings. It's just not a good editing experience. I add them in the API docs like in https://github.com/hynek/stamina/blob/ddb5b19a7e2706a49cbeb4d65352d165f9e444c2/docs/api.md?plain=1#L29-L56
I hadn't realised markdown users were such delicate flowers ;-)
Anyway, taken on board and renamed. I'm going to land this PR now, but if you spot anything else problematic, let me know and I'll try and correct it before the next release.
Well it's more anti-GitHub contrarians I'm thinking of ;)
🤷 - we're all Microsoft customers now, just as intended ;-)
@hynek - Can you have a look over this? In particular the docs. From looking at the
svcs
repo, it doesn't appear you have any example in your source code, your README is in GFM and docs are in Sphinx. Is that right, and is that the way you want it to stay?One thing you may want, which isn't in this PR but will be in the next release, is to have an unconditional skip result in a skipped test:
Currently, unconditional skips only ever result in the skipped example silently passing, which makes it hard to remember to go back and fix them ;-)