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

Proc::Async is generated as Proc/Async #149

Closed
2colours opened this issue Feb 27, 2023 · 21 comments
Closed

Proc::Async is generated as Proc/Async #149

2colours opened this issue Feb 27, 2023 · 21 comments
Assignees
Labels
bug Something isn't working build Building the site

Comments

@2colours
Copy link
Contributor

The documentation of the Proc::Async class used to have a Proc::Async link; now it has a folder-like structure, hence breaking the link.

I don't think it should have a folder-like nature in the first place; it's really just the name of the class. My bet is that this is a content generation problem rather than a link problem.

@coke coke added the bug Something isn't working label Feb 27, 2023
@coke
Copy link
Contributor

coke commented Feb 27, 2023

Regardless of what we consider the "true" links (folders vs. package names) - we have links in the wild pointing to the old style, so we need to support them either directly or with a rewrite rule.

@finanalyst
Copy link
Collaborator

@2colours I'm trying to understand the issue here. Where was the link that Proc::Async used to have? I looked at the Rakupod source. There is a link, eg, to '.start' which works.

Or are you saying that the link to Proc::Async Rakudoc webpage, which is located at Raku/docs/doc/Type/Proc/Async.pod6 should be to /type/Proc::Async.html?

@coke agreed that 'wild' links should be supported by rewrites. We agreed that we would pick up these by looking at web logs that 404.

@coke coke added this to the 2023-Quarter 1 milestone Feb 27, 2023
@coke
Copy link
Contributor

coke commented Feb 27, 2023

we're not going to just rewrite the ones in the 404 log - the log checking was for items where we didn't realize we were missing something - this is a whole class of something that we already knew was missing, now we have a ticket for it. This needs to be fixed soon (it's almost as bad the .html issue)

@finanalyst
Copy link
Collaborator

finanalyst commented Feb 27, 2023

@coke @2colours

this is a whole class of something that we already knew was missing

Would it be possible to make this issue generic, rather than just related Proc::Async?
There is a rewrite rule in the .htaccess file that changes :: to /.
Would this be sufficient?
This issue would therefore be a duplicate of #84

@2colours
Copy link
Contributor Author

@finanalyst I don't know how many similar situations there can be but I see no reason why we switched to "Async in the folder Proc" for something that is really just a class named Proc::Async. That seems conceptually wrong.

@2colours
Copy link
Contributor Author

One more: https://docs.raku.org/type/IO::Path

So yes, to compile my thoughts - I think these "namespaced classes" should just have a flat location in the storage; a Proc::Async.html file for example. That's where the links point to, anyway - without the .html, that is.

@finanalyst
Copy link
Collaborator

@2colours There was no switch.
The Rakudoc source file for Proc::Async is, and has for quite some time, been github.com/Raku/docs/doc/Type/Proc/Async.pod6. If you look at the Type/ directory, you will see lots of files in 'folder' form.

I suspect this goes back to Perl 5, where modules with names like Proc::Async are found as lib/Proc/Async.pm. You will also find many modules continue to do this, eg Proc::Async::timeout, where timeout.pm is at lib/Proc/Async/timeout.pm.

I have had all sorts of problems myself with naming because Raku supports other naming conventions via META6.json.

So the pragmatic answer, but possibly not the purest or most elegant answer, is to ensure that routes to eg. IO::Path go to type/IO/Path.html.

A more problematic ask would be to require for all urls within the Documentation system to be to IO::Path and for the internal link to be actually /type/IO::Path.html. The :: would need to be escaped, I think.

@coke
Copy link
Contributor

coke commented Feb 27, 2023

URL rewriter will work fine for fixing the website issue - we may have to do something about generating the links to ensure we also work with raku/rakudoc, but that's a separate (not yet created) issue.

@coke
Copy link
Contributor

coke commented Feb 27, 2023

Would it be possible to make this issue generic, rather than just related Proc::Async?

This is a generic issue. That's just a specific example

There is a rewrite rule in the .htaccess file that changes :: to /. Would this be sufficient?

We're not using htaccess on the new site, see the recent update that just went live for .html changes.

This issue would therefore be a duplicate of #84

No, this is a specific instance of one of the items in #84.

@coke
Copy link
Contributor

coke commented Feb 27, 2023

@finanalyst I don't know how many similar situations there can be but I see no reason why we switched to "Async in the folder Proc" for something that is really just a class named Proc::Async. That seems conceptually wrong.

This is a common translation - the Proc::Async class is often on disk as Proc/Async.rakumod (it's stored this way in raku/doc source)- As long as we support the original style URL, we don't need to change the build process on the new site.

@finanalyst
Copy link
Collaborator

@coke I think it should be possible to change all the internal links to eg. type/IO::Path and for the html file to be /type/IO::Path.html
This could be done by creating aliases to all Type/{.+}.pod6 files with / replaced by :: (viz ::).

@coke
Copy link
Contributor

coke commented Feb 27, 2023

the caddyshack directive to map the URLs is https://caddyserver.com/docs/caddyfile/directives/uri#uri

change all the internal links

Ok, but it seems like that's a lot of work compared to the URL rewriter for this current issue.

@finanalyst
Copy link
Collaborator

I think for the time being, I would prefer the caddyshack does the mapping. I am unconvinced about making all URLS have ::

@coke
Copy link
Contributor

coke commented Feb 27, 2023

I think for the time being, I would prefer the caddyshack does the mapping. I am unconvinced about making all URLS have ::

It's restoring pre-existing URLs - this shouldn't have been broken.

@2colours
Copy link
Contributor Author

The Rakudoc source file for Proc::Async is, and has for quite some time, been github.com/Raku/docs/doc/Type/Proc/Async.pod6. If you look at the Type/ directory, you will see lots of files in 'folder' form.

Oh okay... well, content-wise, I'd be happier if the file structure didn't suggest contradictory connections to the actual content (as we discussed yesterday, the class Proc and the class Proc::Async probably have less in common than, say, Int and IntStr).

However, that topic can probably go back to the shelf until the calmer, bikeshedding days. :P

@coke
Copy link
Contributor

coke commented Feb 27, 2023

The Rakudoc source file for Proc::Async is, and has for quite some time, been github.com/Raku/docs/doc/Type/Proc/Async.pod6. If you look at the Type/ directory, you will see lots of files in 'folder' form.

Oh okay... well, content-wise, I'd be happier if the file structure didn't suggest contradictory connections to the actual content (as we discussed yesterday, the class Proc and the class Proc::Async probably have less in common than, say, Int and IntStr).

If an issue, that's a core issue with the class naming, not a doc issue, in my opinion.

@finanalyst
Copy link
Collaborator

@coke

It's restoring pre-existing URLs - this shouldn't have been broken.
I agree. My bad. I did not really understand how to do this.

topic can probably go back to the shelf until the calmer, bikeshedding days. :P

I would prefer that.

@coke
Copy link
Contributor

coke commented Feb 27, 2023

@dontlaugh I don't have an easy way to test a change to the Caddyfile locally, but something like

uri /type/* replace :: /

should allow /type/Proc::Async to work again

@coke coke added the build Building the site label Feb 27, 2023
@dontlaugh
Copy link
Collaborator

dontlaugh commented Feb 27, 2023

I can look at this tonight. In the meantime, you can do this to test the Caddyfile

[podman|docker] run -v $PWD/Caddyfile:/etc/caddy/Caddyfile quay.io/colemanx/raku-doc-website

Update: Also, if you have the html and static assets compiled locally, you can mount them into the container under /usr/share/caddy, as well. The add command in our container build script packs the assets under that location at buildtime.

coke added a commit that referenced this issue Feb 27, 2023
go live only supported /Proc/Async, this restores the old functionality - many links to the docs
depend on this style.

Part of #149
coke added a commit that referenced this issue Feb 27, 2023
go live only supported /Proc/Async, this restores the old functionality - many links to the docs
depend on this style.

Part of #149
@coke
Copy link
Contributor

coke commented Feb 27, 2023

Fixed in main, will be available in https://docs-dev.raku.org/ within 30m

@coke
Copy link
Contributor

coke commented Feb 27, 2023

https://docs-dev.raku.org/type/Proc::Async works. Will deploy to prod shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working build Building the site
Projects
None yet
Development

No branches or pull requests

4 participants