simov / markdown-viewer

Markdown Viewer / Browser Extension
MIT License
1.1k stars 136 forks source link

Remote files support #8

Closed jvmlet closed 8 years ago

jvmlet commented 8 years ago

Hi Does this extension support remote files ? For me it works only when I open the local md file with Chrome, remote file (http://host/file.md) is rendered as is. Thanks

simov commented 8 years ago

Yep, here is an example of a markdown file from this repository: https://raw.githubusercontent.com/simov/markdown-viewer/master/syntax.md

simov commented 8 years ago

And here is how it looks like rendered with this extension: markdown

jvmlet commented 8 years ago

Ok, looks like my particular URL is tricky :

URL : http://host:port/path/file.md/*view*/

Response headers:

Accept-Ranges:bytes Content-Disposition:inline; filename=file.md Content-Encoding:gzip Content-Length:2386 Content-Type:text/plain

Any chance to support this ? Thanks

simov commented 8 years ago

Can you provide an actual link to test with, or at least some test location where I can reproduce it?

jvmlet commented 8 years ago

Unfortunately, no, It's on private network, but to simulate this, you can have REST controller that generates the md file and sends it with Content-Disposition response header.

jvmlet commented 8 years ago

BTW, I don't see integration tests in your repository, if you have it, I would be able to PR the controllers' code (preferably with Spring MVC )

simov commented 8 years ago

Currently the extension matches only on:

if (/.*\/.*\.(?:markdown|mdown|mkdn|md|mkd|mdwn|mdtxt|mdtext|text)(?:#.*)?$/.test(tab.url))

That's the first step, and making it match on .md anywhere in the URL will be unreliable.

Only after that the extension can try to make xhr request to that URL and check the headers for Content-Disposition:inline; filename=file.md

So again the first and foremost problem is to reliably determine if the URL should be treated as a markdown file. To get the headers upfront would require completely different set of Chrome APIs and implementation.

jvmlet commented 8 years ago

I totally agree that matching on md extension anywhere in the URL is not the option... Another thought is to have enforce button to trigger the viewer...

jvmlet commented 8 years ago

More sophisticated solution would be to expose the URL matching regex expressions as configuration option, providing your hard-coded

/.*\/.*\.(?:markdown|mdown|mkdn|md|mkd|mdwn|mdtxt|mdtext|text)(?:#.*)?$/

as default.

simov commented 8 years ago

Another thought is to have enforce button to trigger the viewer...

That's not a bad idea. However this will require the extension to be implemented as browserAction instead of pageAction like it is right now. Because right now if the URL is not matched the extension is simply inactive.

But given that the Chrome team abandoned the pageAction's like they used to work, that might not be a huge change here.

Exposing the regex as an option is not a bad idea either, and I'm thinking about implementing an options page at some point.

I'll definitely take into consideration these two options, but I can't give you any estimate on when this is going to be implemented because I want to migrate to Mithril 1.x and implement Material design in the popup first.

simov commented 8 years ago

Actually @jvmlet if you want to help me with this, can you create a small HTTP server that reproduces the behavior you have problems with?

A gist would be great - you can add multiple files in it if needed, and I can download and run the whole thing easily.

jvmlet commented 8 years ago

Let me check If I can host the sample online ...

jvmlet commented 8 years ago

The sample REST service is here : https://md-rest-download.appspot.com/md

simov commented 8 years ago

Thanks!

I got it working but I think that the best way to implement this as a feature is to have an options page where the user can optionally add a matching pattern or something like that.

I'll have to think about it, and probably test a few other ideas as well.

Thanks for the feedback! I'll let you know as soon as I have something.

jvmlet commented 8 years ago

Great, thanks BTW, Markdown Preview Plus solves this issue by making addition ajax request, not sure how it decides when to make it ...

simov commented 8 years ago

Yes, that's a good way to implement it.

I am considering a major change to the permissions required by the extension, and this will affect the way it works. Right now it says: Read and change all your data on the websites you visit which might turn away lots of people.

simov commented 8 years ago

@jvmlet I just published v2.3 this is a really huge update to the extension, but in a nutshell the extension no longer requires any additional permissions for rendering local file URLs.

Any additional remote origin should be enabled manually, here is an example of the new options page:

origins

This page also allows you to set a matching regular expression for the path. Your example server works with the above configuration.

I'll be happy to hear your feedback on this.

jvmlet commented 8 years ago

Looks great , thanks. One note about the origin: it would be nice to have protocol part as optional, or regex. At most cases, user will have to specify two origins for http and https with the same pattern. Thanks again for the great extension.

simov commented 7 years ago

Hey @jvmlet I just published version 2.6 where you can use * to add origin for both https and http. Take a look at the updated docs.