Skip to content

feat: SPA and index.html serving #420

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

Merged
merged 2 commits into from
Feb 27, 2024
Merged

Conversation

Antosser
Copy link
Contributor

@Antosser Antosser commented Feb 25, 2024

Copy/pasted every important bit from the previous PR.

Everything looks great except for one small problem: When the --spa flag is used, the --index flag has to be enabled as well. But the code is really split up into a lot of files and I don't know where I can set cli.index to true if cli.spa is true in the code

Copy link
Member

@LeoBorai LeoBorai left a comment

Choose a reason for hiding this comment

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

Looking better!

@LeoBorai LeoBorai changed the title feat: Implement --spa and --index flags (2nd attempt) feat: Implement --spa and --index flags Feb 25, 2024
@LeoBorai LeoBorai changed the title feat: Implement --spa and --index flags feat: --spa and --index flags Feb 25, 2024
@LeoBorai LeoBorai changed the title feat: --spa and --index flags feat: SPA and index.html serving Feb 25, 2024
@LeoBorai
Copy link
Member

Copy/pasted every important bit from the previous PR.

Everything looks great except for one small problem: When the --spa flag is used, the --index flag has to be enabled as well. But the code is really split up into a lot of files and I don't know where I can set cli.index to true if cli.spa is true in the code

IIRC Clap allows you to have dependencies between arguments.

I think we can merge this and have that improvement as iteration.

@LeoBorai LeoBorai added this pull request to the merge queue Feb 27, 2024
Merged via the queue into http-server-rs:main with commit cf47906 Feb 27, 2024
@Antosser
Copy link
Contributor Author

The problem from the description is still there. In the current state --spa will work weirdly without --index

@Antosser Antosser deleted the indexhtml2 branch February 27, 2024 17:48
github-merge-queue bot pushed a commit that referenced this pull request Feb 27, 2024
If #420 there was still an issue that `--spa would work very weirdly
without `--index`; single page applications are never implemented
without also using `index.html` files. This PR fixes this by
automatically setting `cli.index` to `true` if `cli.spa` is set to true.

I could throw an error if the `--index` flag is not passed in with
`--spa`, but I think those are way too many keystrokes for a feature
someone would already expect

I manually tested this and it works great for every case
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.

2 participants