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

Option to disable file browsing #417

Open
ratulb opened this issue Feb 18, 2024 · 12 comments
Open

Option to disable file browsing #417

ratulb opened this issue Feb 18, 2024 · 12 comments
Assignees
Labels
feature New feature or request

Comments

@ratulb
Copy link

ratulb commented Feb 18, 2024

Description

Could not find anywhere an option to open the index.html page when user opens localhost:7878. How do we do that?

Most of the cases - when we access the server with localhost:7878, we expect to see the home page - not the file system. Of course - file browsing is useful but it should be configurable.

Is there a configuration? Documentation says - by default it is enabled. Did not see an option to disable.

@EstebanBorai
Copy link
Member

Hey @ratulb!

Yes you are right, we need a way to run the server without serving the FileExplorer.

Perhaps to keep backwards compatibilty we could have a --no-explorer?

Let me know your thoughts! And if you are willing toxgive it a shot!

@ratulb
Copy link
Author

ratulb commented Feb 19, 2024 via email

@Antosser
Copy link
Contributor

Antosser commented Feb 19, 2024

I already made a pull request for this exact feature here: #374 . I called the flag--use-index but you can easily rename it to anything else

P.S. Please follow up on the testing framework discussion

@Antosser
Copy link
Contributor

Antosser commented Feb 19, 2024

Hey @ratulb!

Yes you are right, we need a way to run the server without serving the FileExplorer.

Perhaps to keep backwards compatibilty we could have a --no-explorer?

Let me know your thoughts! And if you are willing toxgive it a shot!

I'd go one step further and make that the default. This isn't a library, so it wouldn't be a "breaking" change, and I'd argue that much more people would use this app for html sites than as an explorer in the browser. In other words, I wouldn't be concerned about backwards compatibility

@ratulb
Copy link
Author

ratulb commented Feb 20, 2024

I already made a pull request for this exact feature here: #374 . I called the flag--use-index but you can easily rename it to anything else

P.S. Please follow up on the testing framework discussion

--no-explorer sounds explicit in it's intent without breaking any existing setups.
In a major release later - we could drop this flag all together making it the default behaviour - and could introduce a shorter --explorer flag which would retain current behaviour!

@Antosser
Copy link
Contributor

--no-explorer sounds explicit in it's intent without breaking any existing setups.

I don't think it's necessary for this feature to disable the explorer. Automatically loading index.html in the directory should be enough. If you go to a path without an index.html, why not just fall back to the explorer instead of dropping a 404 page.

@EstebanBorai
Copy link
Member

I already made a pull request for this exact feature here: #374 . I called the flag--use-index but you can easily rename it to anything else
P.S. Please follow up on the testing framework discussion

--no-explorer sounds explicit in it's intent without breaking any existing setups. In a major release later - we could drop this flag all together making it the default behaviour - and could introduce a shorter --explorer flag which would retain current behaviour!

Agree! Its always a good practice to maintain backwards compatibility.
In a second version (perhaps Axum migration), we could consider the proposal by @Antosser IMHO.

@ratulb
Copy link
Author

ratulb commented Feb 20, 2024

--no-explorer sounds explicit in it's intent without breaking any existing setups.

I don't think it's necessary for this feature to disable the explorer. Automatically loading index.html in the directory should be enough. If you go to a path without an index.html, why not just fall back to the explorer instead of dropping a 404 page.

I meant if explicitly not enabled - explorer behaviour should not be available.

command: http-server #server started with explorer feature disabled
Ideally,
localhost:7878 -> Show index.html - if present else 404
localhost:7878/index.html -> Show index.html - if present else 404
localhost:7878/a.html -> Show a.html - if present else 404
localhost:7878/a/b.html -> Show a/b.html - if present else 404

command: http-server --explorer #server started with explorer feature enabled

localhost:7878 -> Show index.html - if present else explorer
localhost:7878/index.html -> Show index.html - if present else explorer
localhost:7878/a.html -> Show a.html - if present else explorer
localhost:7878/a/b.html -> Show a/b.html - if present else explorer

Would like to hear more from you guys.

@Antosser
Copy link
Contributor

I meant if explicitly not enabled - explorer behaviour should not be available.

Why not just leave it in if the path is an existing directory without an index.html? Won't mess anything up and that's how Node's http-server works, so people are kinda going to expect that

@EstebanBorai
Copy link
Member

Hi @ratulb and @Antosser!

I think we have a valuable discussion here, what do you guys think about working together on polishing this feature in @Antosser PR #374 via code review?

@ratulb
Copy link
Author

ratulb commented Feb 26, 2024 via email

@Antosser
Copy link
Contributor

Is this issue now resolved?
Technically, we didn't make a flag, which disables the explorer, but showing a 404 page instead of an explorer if there's no index.html would be less informative and wouldn't be useful

@EstebanBorai EstebanBorai added the feature New feature or request label Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants