search

rome / tools

Unified developer tools for JavaScript, TypeScript, and the web
https://docs.rome.tools/
MIT License
23.74k stars 659 forks source link

☂️ Markdown support #985

Closed sebmck closed 2 years ago

sebmck commented 4 years ago

cc @ematipico and #883

[Edit from @ematipico ]

This is the spec that we should follow: https://github.github.com/gfm/

Current state

The markdown support is slowly resuming and here the things to implement

Linting

What sort of things would we even want to lint in Markdown that wouldn't be solved by autoformatting?

noftaly commented 4 years ago

There is a set of Markdown Linting rules available here if you need inspiration: https://github.com/DavidAnson/markdownlint#rules--aliases.

Some relevant examples:

ematipico commented 4 years ago

no-inline-html

This a good rule but I think we should change it. We want to support GitHub flavored markdown. They support HTML but only for certain tags: https://github.github.com/gfm/#disallowed-raw-html-extension-

ashi009 commented 4 years ago

https://github.com/DavidAnson/markdownlint is great, but the defaults for those rules are less than ideal in terms of minimizing patch size and remove noise from code reviews.

For instance, an ordered list:

1. a
   - sub
2. b
3. c

Will cause multiple line changes when inserts/removes items. However, the following one won't:

1.  a
    - sub
1.  b
1.  c

Also, notice the additional space between the item number and the content, which is to allow indentation alignment when there are nested lists or content.

Do we have a high-level philosophy for making those lint rule decisions? There were lots of debates over such decisions for prettier, and they didn't end well.

tomByrer commented 4 years ago 
* no-inline-html - Prohibits inline HTML

Maybe allows some tags, but not others like <script>

ematipico commented 4 years ago

A couple of things that the formatter could do: spaces between blocks and automatically order a numeric list:

Before

# Title
## Second title
1. First
20. Second

After 

# Title

## Second title

1. First
2. Second
ashi009 commented 4 years ago

@ematipico Please see the comment I made earlier. Automatically order a numeric list is problematic in terms of code review and merge conflict resolution. If rome meant to be configure-less and reasonable, these small things should be considered as well, just like the choice of indentation.

ematipico commented 4 years ago

@ashi009 Sorry, probably I missed your point. Could you explain me better what problems would cause fixing the 20. to 2.? Assuming that there won't be any change in spacing.

Edit: we don't have strong opinions about linting and formatting yet, other tools can be used as inspiration but we don't need to implement them. Our parser will try to follow the spec, which means that 1.__________something and 1._something are exactly the same (consider the _ as space, github was removing them) - by spec.

ashi009 commented 4 years ago

@ashi009 Sorry, probably I missed your point. Could you explain me better what problems would cause fixing the 20. to 2.? Assuming that there won't be any change in spacing.

Edit: we don't have strong opinions about linting and formatting yet, other tools can be used as inspiration but we don't need to implement them. Our parser will try to follow the spec, which means that 1.__________something and 1._something are exactly the same (consider the _ as space, github was removing them) - by spec.

Ha, apparently I didn't make myself clear about the case in the previous comment. Here is a detailed explanation of the case.

Assuming you have a markdown file with the following content:

1. Plan your video content
2. Record your audio and screen
3. Edit the video and ask for feedback

After a while, someone realized that there is an additional step after 1., say 2. Write your script and practice it, now the file looks like:

1. Plan your video content
2. Write your script and practice it
3. Record your audio and screen
4. Edit the video and ask for feedback

Note that, you will have to re-number the entire list to make the numbers continue. Now, they send the change for code review and the diff is all over the file:

 1. Plan your video content
-2. Record your audio and screen
-3. Edit the video and ask for feedback
+2. Write your script and practice it
+3. Record your audio and screen
+4. Edit the video and ask for feedback

Though this change only inserts a single line.

What makes it worse, after a while, a new step 5. Produce and share the video is added to the end of the list, then someone realized that the 2. Write your script and practice it change was unnecessary and would love to revert that change. Unfortunately, git revert won't work without manually resolve the conflict first.

However, if we always use 1. for all items, none of these will happen. Clean diff, and mergeable revert by default. 

 1. Plan your video content
+1. Write your script and practice it
 1. Record your audio and screen
 1. Edit the video and ask for feedback

EDIT: not necessarily a git revert, just consider what happens when someone merges the remote branch. EDIT: use a more realistic example to make this more understandable.

ematipico commented 4 years ago

Oooooh I see your point now! To be honest, I didn't even consider a case like this. Is this what markdowlint does when it lints a file?

What I wanted to suggest was just fixing the numbers based on their position. Doing a sorting based on the text of each list item is something that I really don't want to consider 😆

I was suggesting something simpler:

Before 

1. Z
1. D
5. F
10. FP
9. P

After

1. Z
2. D
3. F
4. FP
5. P

Is it something that would help or would make things worse? Giving your example, I think the only change would be:

1. Do A
+2. Do A2
-2. Do B
+3. Do B
-4. Do C
+4. Do C
ashi009 commented 4 years ago

Oooooh I see your point now!

I think there is still some misunderstanding here.

To be honest, I didn't even consider a case like this. Is this what markdowlint does when it lints a file?

markdownlint can be configured to do so.

What I wanted to suggest was just fixing the numbers based on their position. Doing a sorting based on the text of each list item is something that I really don't want to consider 😆

My example was completely unrelated to the content-based sorting.

I was suggesting something simpler:

Before

1. Z
1. D
5. F
10. FP
9. P

After

1. Z
2. D
3. F
4. FP
5. P

Is it something that would help or would make things worse?

If the existing state is shitty like this, any linter can drive the entropy down.

Giving your example, I think the only change would be:

1. Do A
+2. Do A2
-2. Do B
+3. Do B
-4. Do C
+4. Do C

There are multiple lines of changes here, which is nuts. The diff should be a single line change:

 1. Do A
+1. Do A2
 1. Do B
 1. Do C
ematipico commented 4 years ago

I see now. I'm not sure if we can do something about it. I mean, also GitHub fixes the numeric list automatically and we aim to pair their behavior. Also IntellJ fixes the numeric list.

Here's an example of how Github fixes it: https://imgur.com/a/AHmCSFV

If you wanted to keep the changelog to a minimum, you could have used the bullet point list.

I mean, it looks like that most of the tools already do that.

ashi009 commented 4 years ago

I see now. I'm not sure if we can do something about it. I mean, also GitHub fixes the numeric list automatically and we aim to pair their behavior. Also IntellJ fixes the numeric list.

Here's an example of how Github fixes it: imgur.com/a/AHmCSFV

Github didn't fix it, they render it. The first page you are showing is the markdown source file, the second page is the preview of the generated HTML, which converts whatever number you put in the source file to <ol>s.

Linting happens to the source code, not the generated HTML. We are still not talking.

If you wanted to keep the changelog to a minimum, you could have used the bullet point list.

I mean, it looks like that most of the tools already do that.

Bullet points are for enumerating things that don't care about ordering.

Some additional reading material for you: https://google.github.io/styleguide/docguide/style.html#lists

ematipico commented 4 years ago

Thank you! You made a good point!

darthmaim commented 3 years ago
jbergstroem commented 1 year ago

Is this really completed? 

 $ rome ci README.md
README.md files/missingHandler ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ✖ unhandled file type

Checked 0 files in 196µs
Skipped 1 files

Furthermore, to avoid confusion I'd suggest removing references from the github repository description seeing how doesn't even seem ot be on the roadmap, similar to HTML and CSS (front page, timeline mention).

The Rome Toolchain. A formatter, linter, compiler, bundler, and more for JavaScript, TypeScript, HTML, Markdown, and CSS.

This issue seems to be the best match for people like me who are trying to understand if it is indeed supported or not, hence my comment on an already closed issue. Sorry for the potential noise.

ematipico commented 1 year ago

@jbergstroem this is an ancient issue and I decided to close it because we don't have priority anymore for this feature. We will create a new umbrella once the team will have time again to start works around new languages. For now we have a rough priority:

  1. JSON
  2. HTML
  3. CSS
  4. Markdown

You can refer to the landing page of the website: https://rome.tools/