atom-community / markdown-preview-plus

Markdown Preview + Community Features
https://atom.io/packages/markdown-preview-plus
Other
370 stars 85 forks source link

Version 2.0.0: headings no longer work #88

Closed szhorvat closed 9 years ago

szhorvat commented 9 years ago

Headings with # no longer work with mpp 2.0.0:

image

Headings with "underlines" do work.

leipert commented 9 years ago

@szhorvat Haha, I was just discussing it here https://github.com/atom/markdown-preview/issues/293

Just put a space after the #.

We changed our parser, which tries to follow CommonMark.

CommonMark is mostly compatible with Github Flavored Markdown, at some points it will force you to write cleaner markdown ;)

szhorvat commented 9 years ago

Do you mean that it won't be supported without a space then? That's quite disappointing considering all the MarkDown files I have here (written by me or written by others) that do not have the space ... All other tools I use support this. But then none of them support math well (which was the main feature of mpp).

leipert commented 9 years ago

Let's just ping @Galadirith here.

I will just give you opinion my quickly:

  1. It looks nicer (okay, not a good argument).
  2. I really like the approach of CommonMark. They actually have a good reasoning below this example: http://spec.commonmark.org/0.22/#example-28
  3. http://johnmacfarlane.net/babelmark2/?text=%23TEST
  4. It's really not that dramatic in my opinion, just do a replace with:
^(#{1,6})\s+(.+?)$
$1 $2
Galadirith commented 9 years ago

@szhorvat I do sympathise with you. However I do agree with @leipert with regard to forcing a good standard of markdown with commonmark, and @leipert beat me to the very good motivations for enforcing the spec so it is not something we would fix in MPP.

But Atom is of course all about choice. @leipert gave a great way to perform replacements in files but if you don't fancy @leipert's regex replace you can do a quick an dirty hack to the markdown-it dependency that should work:

  1. Find and open ~/.atom/packages/markdown-preview-plus/node_modules/markdown-it/lib/rules_block/heading.js.
  2. Change L23 from:

    if (level > 6 || (pos < max && ch !== 0x20/* space */)) { return false; }

    to

    if (level > 6) { return false; }

And now you should no longer have the restriction of requiring a space. Thanks @szhorvat I hope that helps, and please ping us back if you wanted to discuss this more :D

leipert commented 9 years ago

@szhorvat Sorry. I am the harsh (german) one, he is the polite (british).

In a future release, maybe 2.1.0 or 2.2.0, we want to support that you can plug in your own markdown-it extensions. With the lines that @Galadirith found, you could easily write a plugin for markdown-it.

szhorvat commented 9 years ago

Let me give you a different perspective.

The main feature that distinguished mpp from other MarkDown previewers is that it has actually usable math support. It is still, as far as I know, the only non-web-based tool like this. This makes it useful as a note-taking tool for academics, and I have recommended it as such to several people. Many of these people don't even know what a regex is, not to mention understanding something as weird looking as ^(#{1,6})\s+(.+?)$ ... If any of them did take up mpp, all they'll see is that suddenly it doesn't render their notes correctly, even though all other tools seem to. If MathOverflow.net renders it fine, why does't mpp? Well, mpp must have a bug.

Some of them might come here and find out that mpp decided to force people to write "correct MarkDown" from now on. Will they have the patience for something like this, especially when mpp seems to stand alone with its insistence on this particular syntax? I strongly suspect that many of the senior MathOverflow members (many professors) won't.

The likely outcome is that these people will just abandon this tool. Perhaps they'll also abandon MarkDown itself after such a bad experience with it, since there's no alternative to mpp that can render math well.

When I upgraded to mpp 2 today, I didn't go back to do find and replace on all my old documents. I just used other previewers for those that don't have much math, and put up with ugly headers in the rest. I'm also not going to do find and replace on any random MarkDown file I come across just to be able to view it. By not supporting such a common syntax, mpp fails as a MarkDown viewer, and is only useful as a tool to write new MarkDown. But then there are many other tools that are useful both as an editor and as a viewer, why would anyone choose the one that's only good for editing, but not viewing?

At this point the only good argument for mpp is its good math support, which means that the people who might choose it are those who find it the biggest burden to keep up with such changes. Breaking their old documents will alienate them too.

So again, who will use mpp then?

This is just my opinion on the matter.

szhorvat commented 9 years ago

@leipert I think you are targeting the tool to young hackers, who have enough time to update their old stuff, to learn how to write plugins, to write plugins, etc. That's a very different audience from the people attracted by the math support.

At this point mpp devs need to make up their mind about what audience they wish to cater to.

leipert commented 9 years ago

@szhorvat Your explanation makes sense. It is not always a cakewalk deciding such things.

I will reopen this issue and if @Galadirith agrees, you will have an option that is enabled by default which allows you to write the headers the "wrong way" by Sunday.

As for the goal audience. I do not really care that much whether certain people use it or not. We will probably break someone's workflow along the way. (Related: https://xkcd.com/1172/) We probably won't make everyone happy.

leipert commented 9 years ago

And a side note:

  1. Thanks for your input on this topic. I hope, we do not loose you as a user, as you did provide a lot of feedback in the past.
  2. May all those :older_man: & :older_woman: who ain't hackin' and slackin' go back to their beautiful LaTeX editors and clay tablets. Just kidding ;)
szhorvat commented 9 years ago

@leipert I surely won't stop tinkering with it! :smile: I just brought it as an example that today when it "broke" in the middle of editing, it was much easier for me to switch to another tool and get on with my work. At that time I didn't know that the fix would have been a few easy spaces.

Also I hope I didn't come through as too harsh above. Take it as one guy's opinion, nothing more.

Galadirith commented 9 years ago

@szhorvat @leipert I would be happy to handle this. So that everyones clear I plan to create a small plugin that will override the default header rule used by markdown-it, which I will probably call markdown-it-lazy-header (lazy's not a judgement just to signify it not following the commonmark spec ;D) then will integrate it as an optional plugin to MPP with it enabled by default (so for all intents and purposes ATX headers in MPP will simply not require the space). It shouldn't take too long to put together, and we'll roll it out this Sunday with any other bug fixes for a 2.0.1 :D

Just regarding what I said earlier

it is not something we would fix in MPP

I suppose I don't really consider this as a fix, as plugins to allow users to extend markdown-it within MPP was always a key motivator for the migration, but I suppose that's just semantics really. However I should have been more careful with my choice of words :D

I hope that will work for you @szhorvat :D And as always please ping us back with any discussion points, its always good to discuss things and its what makes open source great. To chip in with the meme's I always love watching this Steve Jobs response which I think is appropriate in the context :D

Galadirith commented 9 years ago

And not wanting to draw us into another debate but just to briefly comment on @szhorvat

At this point mpp devs need to make up their mind about what audience they wish to cater to.

You know I started MPP because I wanted the best fraking markdown previewer that supported maths, and that mandate hasn't changed. I don't believe we should target any one audience because I really do want this to be useful to everyone hackers and academics alike. I hope that doesn't come off as dismissive of your point but it is something I believe very strongly. But that also means we shouldn't alienate audience's so it should follow Atoms philosophy, usable out of the box but hackable to the core.

Clearly it would be foolish to paint all issues with the same brush. In this case this ultimately will be an easy patch that I have the time to handle providing a large benefit. But I would anticipate not all breaking decisions will be this easy to handle, but that's ok and we'll handle every issue as they come.

I would love to be able to spend time researching ways in which we can provide features that cater to all audiences and anticipate such issues. But even with the incredible support that @leipert has brought to MPP as a collaborator it would take more than the combined time that we have to do that. So that is why people like you @szhorvat are incredibly important, to help shape MPP that is still rapidly evolving by contributing your own thoughts and ideas (just like #88 ;D) and where possible your code ;D to fill that void of time that we can't (and I am quite certain you will also have your own time constraints).

Thanks everyone :D

hemraker commented 9 years ago

To provide a different perspective:

I actually switched to mpp, because of the support for "spaceless headlines" and have been extremely pleased with it so far. So I'm a little bummed to see this development.

For me it is not just a matter being lazy on my part or not wanting to conform to conventions. I get why having the space is best practise but it is not always up to yourself. I work with open source documentation, which means reading a lot of markdown other people have written.

With a huge number of documents, doing search and replace and making changes other contributors documents primarily because I want to use atom to read and write md in doesn't seem reasonable. Especially when a large number of editors/parsers support this. Including GitHub, Stackoverflow (wmd), Ghost and markdowndeep, just to name a few.

I'm not having a go at anyone, just providing some feedback :) Conventions can also be made by numbers. If enough people do it, it becomes the standard.

Galadirith commented 9 years ago

@hemraker Thanks so much for your comment :D

I would have never have thought that spaceless headings would be a motivating feature, so I really appreciate you taking the time to add your opinion :D

As an update for everyone watching, I've already rolled out a new plugin markdown-it-lazy-headers for markdown-it and its working locally for me integrated with MPP. So its will be ready to roll out this Sunday with 2.0.1.

leipert commented 9 years ago

@Galadirith I could also do this one now and we could push 2.0.1 today?

Galadirith commented 9 years ago

@leipert Thats would be great, I suppose I see no reason to wait untill Sunday :D If you wanted to integrate it with a configurable option and merge that would be great. I had a couple of other fixes for 2.0.1, but I'll prepare them locally, and as soon as this is merged into master I'll merge them in and publish 2.0.1.

hemraker commented 9 years ago

@Galadirith @leipert You guys are awesome!! Thanks for a great discussion and a wonderful package :smile:

All the best Rune