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.

Sign up for GitHub

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

Jump to bottom

Adding contentType cli option to address #276 #288

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Adding contentType cli option to address #276 #288

wants to merge 1 commit into from

Conversation

howardroark
Copy link

@howardroark howardroark commented Jun 22, 2016 •
edited by thornjad

fixes #296, fixes #276

@howardroark howardroark mentioned this pull request Jun 22, 2016
Open
@Jameskmonger
Copy link

Hey @howardroark, couple of linting issues on this failed the CI build.

@howardroark howardroark force-pushed the master branch 2 times, most recently from d9bd690 to d127273 Compare June 22, 2016 16:45
@howardroark
Copy link
Author

Thanks @Jameskmonger ! Fixed ;)

@borisirota
Copy link

@Jameskmonger Hi, will this be merged ?

It is very useful in my case where I have static website and prefer the urls to be of a form /about without the extension /about.html.
The files are saved in the file system without the extension but need to be served with the correct Content-Type in order to be parsed correctly.

Just a small note, its not very correct that Use a custom Content-Type response header for all requests.
As I see in the ecstatic source code, the --contentType param is being used only if the mime.lookup fails. IMO its great because it means that this param is a fallback rather than override of expected behavior (e.g. override the Content-Type header of a javascript file to html file's content type).

thornjad
thornjad requested changes Oct 12, 2021
View reviewed changes
bin/http-server
@@ -26,6 +26,7 @@ if (argv.h || argv.help) {
' -s --silent Suppress log messages from output',
' --cors[=headers] Enable CORS via the "Access-Control-Allow-Origin" header',
' Optionally provide CORS headers list separated by commas',
' --contentType Use a custom Content-Type response header for all requests',
Copy link
Member

Choose a reason for hiding this comment

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

We should match the style of other switches by using --content-type

bin/http-server
@@ -26,6 +26,7 @@ if (argv.h || argv.help) {
' -s --silent Suppress log messages from output',
' --cors[=headers] Enable CORS via the "Access-Control-Allow-Origin" header',
' Optionally provide CORS headers list separated by commas',
' --contentType Use a custom Content-Type response header for all requests',
Copy link
Member

Choose a reason for hiding this comment

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

The contentType option actually is used for fallback, so maybe we could clarify by saying something like "Default Content-Type if it cannot be determined"?

Copy link
Member

Choose a reason for hiding this comment

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

possibly even more clear, though a long option, --default-content-type?

@thornjad thornjad added minor version non-breaking, non-trivial change feature labels Oct 12, 2021
@thornjad thornjad mentioned this pull request Oct 15, 2021
Open
@thornjad
Copy link
Member

I know this PR is a few years old, @howardroark I can take this over if you'd like me to

@mmahut
Copy link

mmahut commented Feb 12, 2023

Oh boy.

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

@thornjad thornjad thornjad requested changes

Assignees
No one assigned
Labels
feature minor version non-breaking, non-trivial change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Can I specify the response charset? Add --content-type command line options
5 participants
@howardroark @Jameskmonger @borisirota @thornjad @mmahut