Rake::Site #401

Closed
trans opened this Issue Jul 3, 2012 · 10 comments

Comments

Projects
None yet
3 participants

trans commented Jul 3, 2012

Hi,

I've been using Thin to preview Smeagol websites. Smeagol, btw, is kind of like Jekyll but utilizes a Gollum wiki. Anyway, that's just my particular use case. The problem I'm having though is with serving index pages automatically when a directory path is requested. This is how host provides generally work (or at least allow via .htaccess config), such as GitHub's gh_pages. But to support this, I had to create my own "Site" adapter. Like so:

  Rack::Builder.new do
    use Index, :root=>options[:chdir]
    run Rack::Directory.new(options[:chdir])  
  end

  # Rack middleware to serve an index file by default (e.g. `index.html`).
  class Index
    def initialize(app, options={})
      @app   = app
      @root  = options[:root]  || Dir.pwd
      @index = options[:index] || 'index.html'
    end

    def call(env)
      path = Rack::Utils.unescape(env['PATH_INFO'])
      index_file = ::File.join(@root, path, @index)
      if ::File.exists?(index_file)
        [200, {'Content-Type' => 'text/html'}, ::File.new(index_file)]
      else
        @app.call(env)
      end
    end
  end

I submitted this to Thin project, but they didn't feel it was up to them to provide such adapter. So it occurs to me that really it would be nice if Rack provided for this functionality.

I can submit a pull request for Rack::Index middleware, but it is a "Rack::Site" (the Builder instance above) that would be most useful. But I'm not sure the best way to reimplement this for Rack. How would I create a Rack::Site class akin to Rack::File and Rack::Directory that handles this?

(Note, another option would be to allow Rack::Directory to support this as a configuration option, but I don't see how its interface would allow for supplying the config option, so I figure creating a new class instead is probably best.)

(Also note, that I know I need to adjust the code to change the content type based on the type of index file. Haven't done that yet.)

Contributor

elcuervo commented Jul 3, 2012

I think that what you want is achievable via Rack::Static https://github.com/rack/rack/blob/master/test/spec_static.rb#L19

trans commented Jul 3, 2012

Ah, thanks for reminding me about that. I should have mentioned it too. I looked into that before and Rack::Static doesn't work either. It doesn't actually look to see if the reference is a directory but just looks for a trailing '/', so it doesn't work in all cases. Perhaps this should be considered a bug though? Can I submit a patch that would fix this?

Contributor

elcuervo commented Jul 3, 2012

Did you try this?: https://github.com/rack/rack/blob/master/lib/rack/static.rb#L25-30

elCuervo
http://elcuervo.co
http://github.com/elcuervo

On Tuesday, July 3, 2012 at 4:18 PM, 7rans wrote:

Ah, thanks for reminding me about that. I should have mentioned it too. I looked into that before and Rack::Static doesn't work either. It doesn't actually look to see if the reference is a directory but just looks for a trailing '/', so it doesn't work in all cases. Perhaps this should be considered a bug though? Can I submit a patch that would fix this?


Reply to this email directly or view it on GitHub:
#401 (comment)

trans commented Jul 5, 2012

ping Can I do ahead and patch? Should it be another option? Or should I just modify current behavior?

Contributor

elcuervo commented Jul 6, 2012

Sorry about missing the last comment.
That is working, but remember that it's master. There was no release with that feature yet.

trans commented Jul 6, 2012

I reviewed the master source on github, I don't see how it can be working from what I see there. But I will try it try and get back.

Contributor

elcuervo commented Jul 6, 2012

Oh, I've missed the part where you want to access /foo/ as well as /foo. Sorry about that.

trans commented Jul 6, 2012

No problem. Basically all I'd like to do is add an if File.directory? it the check for trailing / comes out false. I can make that standard behavior or I can add another option that needs to be set to true that allows that check to happen.

Owner

raggi commented Nov 2, 2012

You have a PR open for this, we can track this there. I'll close the ticket.

@raggi raggi closed this Nov 2, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment