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

Problem (and solution) for sites in subdirectory #23

Closed
leogermani opened this issue May 9, 2019 · 10 comments · Fixed by #34
Closed

Problem (and solution) for sites in subdirectory #23

leogermani opened this issue May 9, 2019 · 10 comments · Fixed by #34

Comments

@leogermani
Copy link
Contributor

Hi,

Great plugin!

I found an issue when using it in a website that is not in the root folder of the web server. All the links to the endpoints in the documentation would point to /api-docs/.... So if I was visiting the docs in localhost/my-site/api-docs, for example, all links would point to localhost/api-docs.

It took me a while even to notice that, because as all the interaction happens in the React app, the links actually worked. The contents were correctly loaded, even though the address in the address bar would change to an unexisting one.

But after I fixed it, I realized that all the content from the default pages were not being loaded.

So after tweaking a bit, I came up with the solution below.

In Restsplain.php, line 73, where you read:

	$config = array(
		'basename'          => get_docs_base(),

I changed to:

	$base_url = parse_url( home_url() );

	$config = array(
		'basename'          => $base_url['path'] . get_docs_base(),

I really don't know the implications of that, Ive being playing with the plugin for an hour or so, but it seems to work fine.

If you had the time to have a look and see if it looks good it would be great to see this fix incorporated. I'd be happy to send a pull request or feel free to implement it directly.

cheers

@roborourke
Copy link
Contributor

Hey, that look sensible to me. It could even be done in the get_docs_base() function directly. Are you able to make a pull request for this? Sorry for the long delay in replying, I have too much other stuff on and this hasn't been a priority as it's still mostly functional.

@leogermani
Copy link
Contributor Author

sure, will do!

leogermani added a commit to leogermani/Restsplain that referenced this issue Jul 21, 2021
@leogermani
Copy link
Contributor Author

Thanks for looking at it.

as it's still mostly functional.

I guess the most critical problem is #26

@WillyMichel
Copy link

Hi guys,
Just an update on this fix. In some cases I don't have the 'path' returned in the parse_url() function. So I changed it like this (php8) :

$base_url = parse_url( home_url() );
$base_url_path = rtrim( $base_url['path'] ?? '', '/' );
$config = array(
	'basename'          => $base_url_path . get_docs_base(),

@leogermani
Copy link
Contributor Author

Good point, but:

  • we should return an empty string if there's no path, otherwise we will end up with two slashes in a row when we concatatenate it
  • I'm not sure what versions of PHP we want to support. By default I tend to follow what WordPress supports

@leogermani
Copy link
Contributor Author

updated the PR

@WillyMichel
Copy link

Thanks
The rtrim should avoid double slashes
I mentioned php8 because i'm using it, but the piece of code I pasted is php7+ I think

@roborourke
Copy link
Contributor

PHP7+ is fine, just wanted to note you could use parse_url( home_url(), PHP_URL_PATH ) to return only the path component of the URL, pretty sure that'll give you an empty string if there isn't one.

@leogermani
Copy link
Contributor Author

parse_url( home_url(), PHP_URL_PATH )

This looks way better. Updated the PR

@WillyMichel
Copy link

Indeed it is better :)

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 a pull request may close this issue.

3 participants