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 Table Tag Support: #32

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add Table Tag Support: #32

wants to merge 1 commit into from

Conversation

BuilderQiu
Copy link

Translate Markdown :

titleA|titleB|titleC|titleD
---|---|---|---|---
a|b|c|d
1|2|3|4
...

To Html:

  <table>
   <thead>
    <tr>
     <th>titleA</th>
     <th>titleB</th>
     <th>titleC</th>
     <th>titleD</th>
    </tr>
   </thead>
   <tbody>
    <tr>
     <td>a</td>
     <td>b</td>
     <td>c</td>
     <td>d</td>
    </tr>
    <tr>
     <td>1</td>
     <td>2</td>
     <td>3</td>
     <td>4</td>
    </tr>
   </tbody>
  </table>

Translate Markdown :
titleA|titleB|titleC|titleD
---|---|---|---|---
a|b|c|d
1|2|3|4
...

To Html:

  <table>
   <thead>
    <tr>
     <th>titleA</th>
     <th>titleB</th>
     <th>titleC</th>
     <th>titleD</th>
    </tr>
   </thead>
   <tbody>
    <tr>
     <td>a</td>
     <td>b</td>
     <td>c</td>
     <td>d</td>
    </tr>
    <tr>
     <td>1</td>
     <td>2</td>
     <td>3</td>
     <td>4</td>
    </tr>
   </tbody>
  </table>
@rjeschke
Copy link
Owner

After a quick look, I do have some major concerns:

  • have you compared your formatting with the rest of my code?
  • using line.next.next without checking if line.next is != null is a bad idea
  • refrain from using regex (you may have noticed that I do not use a single one), and even when I allowed regex's, they should be compiled
  • <tableHead> isn't even valid HTML
  • a table is not standard markdown, so it should not be parsed when not in extended mode
  • don't document new Markdown features with code comments, if it's a new feature it belongs into README.md, basically all of your comments (except the JavaDoc ones) can be removed

@BuilderQiu
Copy link
Author

Thanks for your reply,and something I want to know:

  • did you means that all code committed need the same formatting?I found the different between your code and mine before I committed.
  • this place I can make sure that line.next is not null,so I am not check it
  • I want know why you not use regex in your code?because of performance or other?
  • did you means all not standard markdown could not in this place, and need add in any other place?

@@ -296,6 +296,12 @@ public LineType getLineType(final Configuration configuration)
return LineType.BQUOTE;
}

if(this.leading == 0 && this.value.matches("(.+\\|)+.+") && this.next != null && this.next.value != null && this.next.value.matches("(-+\\|)*-*")){
Copy link

Choose a reason for hiding this comment

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

Table in Markdown may be better to be like

title1 title2
value1 value2

It's easy to parse and more pretty

Choose a reason for hiding this comment

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

Please refer: https://www.tablesgenerator.com/markdown_tables to generator markdown table.
The first line and second line should be:

| Tables   |      Are      |  Cool |
|----------|:-------------:|------:|
if (this.leading == 0 && this.value.matches("^\\|(.+\\|)+.+\\|$")){
      // First Line Looks Like:  | Tables   |      Are      |  Cool |
      if (this.next != null && this.next.value != null && this.next.value.matches("^\\|(-|:|\\|)+\\|$")) {
            // Second Line Looks Like: |----------|:-------------:|------:|
            return LineType.TABLE;
      }
}

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

Successfully merging this pull request may close these issues.

4 participants