Add info field to Code block by arobase-che · Pull Request #22 · syntax-tree/mdast (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation25 Commits2 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

arobase-che

Hi :)

It's just an update of the Code's description. The main modification is the info string tag.

Double check my english please. 🙏

💕

wooorm

Choose a reason for hiding this comment

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

Looks very nice! One thing I’m wondering is why not just use info instead of infoString?

readme.md Outdated Show resolved Hide resolved

@arobase-che

:)

No reason, it's as you wish

@Hypercubed Yeah I was thinking something along those lines, a code.infoString being the whole thing, lang being the first “word”? It’s breaking, but more fixing.

So i take infoString, in first place i taked "info-string" and it's definitively ugly.

@Hypercubed

I would think that since the common mark spec refers to it as the info string the property should be infoString... but it doesn't bother me either way.

@arobase-che

For the language, it's "lang" so for the info string, the tag "info" doesn't seen weird.

For the request, it's done (for the attribute's name too btw)

wooorm

@wooorm

Yeah I think I prefer info over infoString. Feels a bit weird to add the string prefix, whether commonmark does it or not!

@wooorm

Say we have this:

fn()

And we have the node for that code:

{ type: 'code', info: 'js no-highlight', lang: 'js', value: 'fn()' }

...what if someone changed node.lang = 'c' or something else?

@arobase-che

I think that info should be the whole string. The main reason is that the definition of info string from GMF and CommonMark is the whole string. An other is that it's common to specify the language as the first word but it's not specified.

Spec :

The first word of the info string is typically used to specify the language

But as you notify, it can lead to inconsistent states.

PS: Oups ! I miss clicked on close and comment ... >_<"

@wooorm

I’d rather not specify something that could create inconsistencies, and IMO it makes sense to see:

to

{ type: 'code', lang: 'ruby', info: 'startline=3 %@#' }

...note, btw, I don’t think commonmark specifies what a “word” is in:

The first word of the info string is typically used to specify the language of the code sample, and rendered in the class attribute of the code tag. However, this spec does not mandate any particular treatment of the info string.

@arobase-che

:(
Ok

So it's more complicated. Now it need specifications 😢

So ... here we go !

lang='js' info='info'

Trailing spaces before and after the info string are skipped

lang='js' info='info'

No info string is null

lang='js' info=null

Is it ok like that ?

Edit: Easy spec, i don't want it to be too complicated. Can't handle complicated things.

@wooorm

Yeah I think this is fine! I don’t believe we should care about spacing between lang and info. I believe those could be stripped away. We may need to work out what a “word” means though. What do you think?

@ChristianMurphy Any thoughts?

@ChristianMurphy

Trailing spaces before and after the info string are skipped

So it would trim external space but not internal?
E.G.

would parse to

{ "info": "something that has random spacing" }

rather than

{ "info": "something that has random spacing" }

?

lang='js' info=null
Is it ok like that?

Sounds good

@arobase-che

Actually here, we don't really care about words but the most important is that it can handle programming language names.

That to Wikipedia, programming languages list.

In the code i defined it by every char until a white space (\s) or a curly bracket (but we may add every kind of bracket).

Yeap, it will trim extrernal but not internal spaces. Or maybe no trim at all ? The choice is yours.

@wooorm

@arobase-che

@wooorm ? Border ? An example please ? Only white-space you mean ?

Triming internal spaces can be tricky.

@wooorm

@arobase-che I mean the “border” between lang and info. And I believe internal white-space should be kept as is. Just the “border” should be removed.

@arobase-che

Ok so :

lang='golang' info='info'

But :

lang='go{info}' info=null

It become a problem with that :

~~~ go( this is a comment )
~~~

:(

Sorry if I don't understand you. We keep all characters until the first white-space (white-space excluded, so the white-space is the border no ?). The rest of the text is the info attributes and we trim it out (so the border is removed ?)

I believe too that internal white-space should be kept.

@ChristianMurphy

@wooorm

Yes, I believe that:

Should turn into: "js", "something", "js", "something", "go(", "something)", "go(", "s·om····ethi···ng)"

@arobase-che

@ChristianMurphy None i think. It's about the user's expected result i think. Some may want to use Kramdown Attribute lists.

No language has brackets in its name. So why make a error consciously ?

I will code it in any case but ... :s

@ChristianMurphy

None i think. It's about the user's expected result i think

Maybe it could be an option or a plugin that could be enabled.
As a user of remark myself, I would find output that doesn't match commonmark unexpected.

@arobase-che

Ok, makes sense. Even if i think that CommonMark is wrong.

See you soon people ^^

@arobase-che

@arobase-che

@arobase-che

I updated the description and the PR on remark. ^^

But travis still buging 😢 remarkjs/remark#345

@ChristianMurphy

It probably started during the minor Travis CI outage.
Restarted bugging travis instance, it passed this time.

ChristianMurphy

@arobase-che

wooorm pushed a commit that referenced this pull request

Aug 20, 2018

@arobase-che @wooorm

Closes GH-22.

Reviewed-by: Christian Murphy Christian.Murphy.42@gmail.com Reviewed-by: Titus Wormer tituswormer@gmail.com

wooorm added a commit that referenced this pull request

Aug 20, 2018

@wooorm

wooorm added a commit to remarkjs/remark that referenced this pull request

Oct 6, 2018

@arobase-che @wooorm

Previously, the info string of a code block, was not parsed at all, and was stored in lang on code nodes. Now, the first “word” of the info string is stored in lang, whereas the rest is stored in meta.

Related to syntax-tree/mdast#22 (PR) Closes GH-342 (issue) Closes GH-345 (PR)

Reviewed-by: Titus Wormer tituswormer@gmail.com Reviewed-by: Christian Murphy christian.murphy.42@gmail.com

Co-authored-by: Titus Wormer tituswormer@gmail.com

@wooorm wooorm changed the titleUpdate description of Code Add info field to Code block

Aug 12, 2019