search

kates / html2markdown

Converts HTML to Markdown
MIT License
132 stars 48 forks source link

add normalize URL as a configurable feature #6

Closed sudodoki closed 10 years ago

sudodoki commented 11 years ago

Adding feature to optionally normalize URL's.

kates commented 11 years ago

Hey @sudodoki, can you indent by tabs? I'll merge this after.

Thanks.

sudodoki commented 11 years ago

@kates I think I can. :smile:

sudodoki commented 11 years ago

Didn't run the actual tests in window-less env, but ran the snippet in the node environment - seems legit, tell your thought on this. Even better way to do this would be by providing getBase() function and using it both in method and tests and just mocking it, but working with real window.location appeals more to me

sudodoki commented 11 years ago

It's just really bad way to hardcode the value. I did spend some time figuring out, why test on previous revision (the one you had normalizeUrl functionality) failed, and didn't notice the port thing (I was running on another one on localhost) at once. Don't think I'm trying to ruin tests or something, you did an awesome job I wanted to contribute to. Kthxbye

kates commented 11 years ago

@sudodoki You did good man. It's just that, I think, it doesn't matter if it's run in the browser or in any other environment. The main thing is we shouldn't muck with the document data. We should be able to convert back to HTML without or, at the very least, have minimal lost of data. In you example, it will change the link to a full url given just a relative url. Converting it back to HTML will yield a full url since we have already lost context.

sudodoki commented 11 years ago

The use case I'm willing to cover is provided html2markup is ran in the context of, say, current html page and then markdown is being transfered to another host, I want it to be with actual images after reverse convertion (with images from another host) and not with 404s.

sudodoki commented 10 years ago

Should I modify this or just close the PR?