rf / nd

a documentation viewer for node
MIT License
86 stars 4 forks source link

melded-in dependency: marked #10

Closed StreetStrider closed 9 years ago

StreetStrider commented 9 years ago

Hi. I've found deps/marked in repo and it seemes to be used: app.js. Looks like marked is present at npm, it's popular and supported. Maybe this should be moved to dependencies? Is there any special reason this done in such manner?

rf commented 9 years ago

It's been a few years but from what I remember I added terminal output to marked but @chjj didn't want to merge it in, so I just threw it in a deps dir. I did put it in a repo but I never published it.

If you want to publish markedterm or try to get it merged feel free, I'll take the PR, but seeing as how it works and all I'm not too concerned.

StreetStrider commented 9 years ago

@rf, all clear now, thanks. Well, npm supports git-endpoints and even GitHub-shortcuts. Like "marked": "rf/markedterm" (will be resolved to GitHub git-endpoint of your repo). So you don't need to be bothered by publishing it in central repo and maintaining such package. For stability it also can be fixed with git-tag, like "marked": "rf/markedterm#v0.2.5".

rf commented 9 years ago

True, I could do that too but it's not much better than keeping it in a deps folder. It'll be pinned to a tag, so we don't get semver benefits. But if you wanna make the PR I'll merge it. If there's new functionality in marked that you want then bonus points if you rebase my changes and bring markedterm to parity with upstream marked. But it's been 3 years so that might not be a viable solution anymore.

StreetStrider commented 9 years ago

@rf, I think keeping dep in another repo (especially if the repo already exists) is a sane thing to do. For now I don't understand completely what you've implemented in marked and why maintainers reject it, but I'll investigate it too.

rf commented 9 years ago

It's just a little 'output plugin' that uses ansi escapes rather than html tags (lib/terminal.js). marked is really a markdown to html renderer. And I suppose this functionality isn't often needed enough for Chris to want to take it in and maintain it, which I can understand. Or maybe my code is just really bad.

StreetStrider commented 9 years ago

@rf, ok, thanks, this sounds reasonable. I'll see what can be done there.

StreetStrider commented 9 years ago

@rf, you was right. I've compared nowadays marked and your fork and there is a big divergence between them. New abstractions was added, but lib is still in one monolythic file, so I can't merge it without deep research.

StreetStrider commented 9 years ago

Have and interesting idea, though. It is possible to just use default marked (or other markdown parser), but then transform html to cli ansi codes. It sounds silly, but it allows to view markdown files mixed with HTML tags, like was mentioned in #3. So workflow would be: markdown → html → cli → pager. If html to cli transformer exists, then implementation would be a simple composition of this parts. Have found package hermit, maybe other exists as well. This is just an idea.