Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mime type for HTML files and support index.html #109

Merged
merged 1 commit into from
May 22, 2014
Merged

Mime type for HTML files and support index.html #109

merged 1 commit into from
May 22, 2014

Conversation

isotes
Copy link
Contributor

@isotes isotes commented May 21, 2014

I have read #93 and in general agree wholeheartedly with the Unix philosophy. Nevertheless, since it's just 2 lines, it would be great to also serve html files with the correct mime type, and handle index.html files.

Note that this means the browser has to be directed to, e.g., http://localhost:35729/foo/bar.html

At least for my personal use case that consists mostly of js and css development (without any ruby server technology), this makes it considerably easier to get started.

@thibaudgg
Copy link
Member

The need to go on http://localhost:35729/index.html looks a bit hackish to me, are you sure that's a good idea.
I would like to keep the separation of concern and I prefer to not opening that door.
I think it would be better to load your html file from a separate server (that's so easy!).

@isotes
Copy link
Contributor Author

isotes commented May 21, 2014

Last try, I promise :)

Since all development servers require a special port (8080, 8888, ...) - and the default settings vary from server to server - I do not think that having to specify the port 35729 in the browser makes it more complicated than running a dedicated server and having to specify its port. And the first line in the patch makes it possible to omit the index.html as well, so it boils down to http://localhost:35729.

Regarding the ease of a separate server: perhaps it is just a problem for me because I have no Rails background (or need) at all, but when I came across this (really great) project some time ago, I've read the Readme, went over to the referenced rack-livereload site, saw the line Then add the middleware to your Rails middleware stack... and got stuck.

As a reference, my current setup looks usually like this, which is probably hard to beat for simplicity:

# Gemfile
source 'https://rubygems.org'
group :development do
    gem 'guard-livereload', :require => false
end

# Guardfile
system "open http://localhost:35729" # opens URL in browser on OS X
guard 'livereload' do
    watch(/.*\.(html|js|css)/)
end

Regarding the separation of concerns: I of course see that point. The only reason why I ended up adding it, was that almost everything is already there: even html files are already served, just with a wrong mime type

@thibaudgg
Copy link
Member

Ok, I got your point. I think it'll adding these too line will not broke the world! Thanks for your feedback.

@@ -12,6 +12,7 @@ def dispatch(data)
parser << data
# prepend with '.' to make request url usable as a file path
request_path = '.' + URI.parse(parser.request_url).path
request_path += '/index.html' if File.directory? request_path
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be request_path += './index.html' here? (a dot is prepend the line before)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's say, there is a file foo/index.html and a request for /foo.
The first request_path line makes it ./foo, i.e. a local relative file path.
The second request_path line then makes it ./foo/index.html. With the line from your question it would be foo./index.html which does not exist.
The current version also works with the request /foo/ (resulting in ./foo//index.html, which still works) and / (resulting in ./index.html)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok great, gotcha!

@isotes
Copy link
Contributor Author

isotes commented May 22, 2014

Thanks a lot!

thibaudgg added a commit that referenced this pull request May 22, 2014
Mime type for HTML files and support index.html
@thibaudgg thibaudgg merged commit 3d01dad into guard:master May 22, 2014
@thibaudgg
Copy link
Member

You're welcome, 2.2.0 released!

@isotes isotes deleted the serve-html branch September 16, 2015 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants