asciidoctor / atom-language-asciidoc

⚛ AsciiDoc language package for the Atom editor.
https://atom.io/packages/language-asciidoc
MIT License
42 stars 20 forks source link

Enhance tables support #104

Closed ldez closed 8 years ago

ldez commented 8 years ago

Related to #3

nicorikken commented 8 years ago

Any findings on this topic already?

ldez commented 8 years ago

Pinky: Gee, Brain, what do you want to do tonight? Brain: The same thing we do every night, Pinky - try to take over the world!

ldez commented 8 years ago
foo *foobar* bar

|===
 |1      >s|2   |3        |4
^|5 2.2+^.^|6      .3+<.>m|7
^|8
 |9     2+>|10
|===

foo *foobar* bar

capture du 2016-05-14 01-10-29 capture du 2016-05-14 01-10-49 capture du 2016-05-14 01-11-25 capture du 2016-05-14 01-12-03 capture du 2016-05-14 01-12-19

mojavelinux commented 8 years ago

I'm really looking forward to this one!

mojavelinux commented 8 years ago

It seems like only the Atom Dark theme adds color to the table and table cell delimiters. How is it that all your examples have color?

I think we should distinguish between table (the delimited block) and table-cell (the data separator). Perhaps we can add the name .punctuation.separator to the separators. (taking a page from the gfm grammar).

mojavelinux commented 8 years ago

I don't see any mention of dsv or csv in the grammar file, but I see you checked these off in the list above. Have you just implemented psv?

ldez commented 8 years ago

I think you are not up-to-date with my branch.

  tables:
    patterns: [
      {
        include: "#table-standard"
      }
      {
        include: "#table-nested"
      }
      {
        include: "#table-csv"
      }
      {
        include: "#table-dsv"
      }
    ]

Can you retry to chechout my branch ?

ldez commented 8 years ago

Maybe you have reloaded grammar without reload the code: for this PR, if your Atom is running before the checkout of this branch, you must press ctrl+shift+R

mojavelinux commented 8 years ago

Oops! My bad. Sorry for the static. I'll update.

ldez commented 8 years ago

For CSV, do you want me to add the support for rfc4180 ?

NOTE: I have created a brand new package for Atom (language-csv) which support CSV grammar :wink:

mojavelinux commented 8 years ago

Indeed, it would be brilliant if the content of the CSV table was treated like a CSV source block. That way, we delegate all that away to the other grammar package (which could be optional).

mojavelinux commented 8 years ago

I updated my branch and this looks great.

I think we should discuss one issue pertain to naming:

I also wonder if we should combine table and nested table for performance reasons. I don't think it's so important to label the nested table as nested. wdyt?

ldez commented 8 years ago

I want to have same style for the table delimiter and the cell delimiter, it's more readable.

If I use punctuation.separator.cell.asciidoc:

capture du 2016-05-15 00-25-49

mojavelinux commented 8 years ago

I agree that would be nice, but then we are changing semantics to match existing style choices. I think it's more correct to fix the styles. I'm quite certain in this case that punctuation.separator is more correct than constant.other.symbol.

I'd argue that the table delimiters should be punctuation.definition.table or something.

ldez commented 8 years ago

I'm not agree to provide custom style but I will found a compromise.

Customize the style of an existing code like punctuation.separator not seems to me a good idea, I prefer create a dedicated code like markup.table.delimiter and markup.table.delimiter.cell.

wdyt?

ldez commented 8 years ago

Done!

nicorikken commented 8 years ago

Nice, works, and as I saw no further comments in the previous days, I'll merge this one.