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

Add baseDir option, add -b flag to cli options to specify base path #294

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jesseditson
Copy link

@jesseditson jesseditson commented Jul 19, 2016

node-ecstatic allows for a baseDir option, which I was looking to use with this lib.
This PR adds the following:

  • a baseDir option to the main lib file
  • a -b flag to the cli tool
  • a path variable to printed/opened URLs
  • CLI and README docs for baseDir and flag
  • tests for new baseDir option

Fixes #139

@miguelrincon

This comment has been minimized.

@wootsaejao

This comment has been minimized.

@jesseditson
Copy link
Author

Rebased in case this becomes mergeable - @indexzero LMK if I should keep this branch in sync or if we should close this PR, thanks!

@hikumealan
Copy link

This is the same as #395 - hope the feature is available soon.

@kolisko

This comment has been minimized.

@BigBlueHat BigBlueHat added the major version Major, potentially breaking, change label Jun 22, 2018
@BigBlueHat
Copy link
Member

This needs rebasing on top of the latest from master, but I generally prefer it (because tests, etc) to what's in #395.

If someone wants to get this up to speed with the latest from master, I'm 👍 on merging it.

@BigBlueHat BigBlueHat added minor version non-breaking, non-trivial change and removed major version Major, potentially breaking, change labels Jun 22, 2018
@jesseditson
Copy link
Author

All set. Noticed the paths weren't printing to the logs, so I added that too.

@BigBlueHat
Copy link
Member

Hrm... -b conflicts with #463. @thornjad we should be careful about our shorthand choices probably... #395 also used -b. Thinking baseDir setting will be more frequently used, so maybe we use -b here, and remove the alias from #463.

@thornjad
Copy link
Member

@BigBlueHat yeah that makes sense, 'b' is a popular letter. Brotli can be --brotli only

@thornjad
Copy link
Member

Alright, @idmontie's #395 and @jesseditson's #294 have a one-line difference. I'm going to go with #294 because it came first

Copy link
Member

@thornjad thornjad left a comment

Choose a reason for hiding this comment

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

This branch is a bit behind master, and there are a few change still scheduled (like #131) which will happen before this PR merges

test/http-server-test.js Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
bin/http-server Outdated Show resolved Hide resolved
bin/http-server Outdated Show resolved Hide resolved
@jesseditson
Copy link
Author

I'll fix this up tonight (PST) and rebase against master!

@thornjad thornjad added this to the v0.12.0 milestone Apr 22, 2019
@jesseditson
Copy link
Author

Ok I think this is up to date, LMK if there's anything else needed on my end!

@thornjad thornjad modified the milestones: v0.12.0, v0.13.0 Nov 19, 2019
@thornjad thornjad self-requested a review December 11, 2019 17:33
@culmat
Copy link

culmat commented Feb 2, 2020

Would it be possible to split the change into

  1. introduce feature and --base-dir flag in a 12.x minor version
  2. default -b from --brotli to --base-dir in 13.x

The implementation seams ready and the functionality is requested ( see #139 ) since years.
My use case is SPA with client side routing in 404.html on github pages.
It would love to clean up my code ( dev mode work around ) as soon as this is available.

@bishtawi
Copy link

Is this PR going to be merged any time soon? Would love to be able to use this functionality.

@cattermo
Copy link

cattermo commented Jun 2, 2021

Oh, please, is there any way one can help to get this merged?

@thornjad
Copy link
Member

thornjad commented Jul 12, 2021

Requirements for merging:

  • Add documentation to doc/http-server.1
  • Wait for Ecstatic with tests #693 to merge (because it will likely cause conflicts)
  • Fix all conflicts

@thornjad thornjad modified the milestones: v0.13.0, v14.0 Aug 7, 2021
@jesseditson
Copy link
Author

This has long fallen off of my radar (I filed this PR 5 years ago and no longer have a dependency on this lib), happy to fix conflicts if a different interested party wants to get the docs going.

@thornjad thornjad removed this from the v14.0 milestone Oct 11, 2021
@abbeysside
Copy link

Is this still an open feature?

I was trying to implement http-server with a base path but the -b flag is used for brotli now...

@MaximSagan
Copy link

@thornjad @jesseditson
As this PR is 6 years old and (as one might expect) has gone stale, I rebased, fixed merge conflicts and made a new PR here: #837

@hairinwind
Copy link

@thornjad @jesseditson As this PR is 6 years old and (as one might expect) has gone stale, I rebased, fixed merge conflicts and made a new PR here: #837

Can this be merged? We do need this feature. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor version non-breaking, non-trivial change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Serve multiple directories